Deprecate Project.documentation_type

[agjohnson]

Our config file is going to support per-version documentation types, so we need to make a few changes to support this logic. See #4486 for some background here. The two points where we are still using Project.documenation_type are in the footer_html return and in our resolver.

We can't remove the need for the data from this field, as we have some logic that determines file path based on the build backend type. So, we have two options:

Remove documentation type completely

Is there another way to remove this logic or not depend on a Version.documentation_type?

I think we are 👎 on this method as it may not be possible without removing the path logic. If there is a way, this is the easiest though.

Move to a generated Version.documentation_type

A couple options for this field:

• Make it a model field
• Make it a property that uses the last successful Build.documentation_type instead

My vote is maybe for option 2. We already use Build as a writing point for build metadata. Updating a documentation_type from the config would make sense here. We also talked about adding a general Build.config for storing the used config on the build. Perhaps this work can be combined here. Perhaps cache this property as well?

Pieces that will be or could be changed here:

• Add a Build.config json blob for storing config meta data (separate PR?)
• Make build update Build.config json blob
• Make Version.documentation_type property
• Move logic for determining path to Version?
• Tune footer_html template to not use main_language_project.get_url() + path, but instead use something closer to Version.get_url_for(path). Logic should get the per-version URL, not kludge together a URL.
• Rely on self.config for change in Put search step back into project build task #4655

[stsewd]

Just linking the commit that we'll need to revert later 80e3904

[stsewd]

• Add a Build.config json blob for storing config meta data (separate PR?)
• Make build update Build.config json blob

Those looks the same for me p: do you mean on 1) making something like we do with the build output? (store json files instead of the db) and 2) is to store on the db as a json field?

Anyway, I'm +1 for storing this information under a metadata field for the Build model, as we can use that information for other stuff (like show the final configuration used, suggest using a configuration file, etc)

[agjohnson]

Sorry, no, i meant we need to add a field or fields to Build and the second part is making the API update these pieces on successful build.

If we don't have other metadata, we can also use a one-off field. But I think we're leaning towards storing more metadata (it would help greatly with debugging and UX around build).

[stsewd]

So, I re-read and think I get it now p: I'm +1 for using a json field. Do we want to do something like output field (it's saved in other location as a json file)?

If we don't want to complicate things, I'm fine with one field too.

[agjohnson]

No, we don't want to store this outside the build as we'll be using the data directly. Build command output goes to cold storage eventually as it's really only user facing output and is probably rarely accessed compared to how often we're storing it.

This would just be a json field on build model I think. If there is a reason to query any of this data, that might push us towards another model mechanism, but django-jsonfield at least has some ability to be queried.

[stsewd]

So, I was digging a little here about removing Project.documentation_type completely.

• Previous projects don't have the config saved (we can do a migration here, just update all previous builds of earch version to the doctumentation_type of the project).
• People that want to build their docs with other than sphinx, have to use a config file.

If we don't want to completely removed it:

• We can have a property in version, wich will default to the project doctype if we don't have the doctype in the config field of the build (or we can still do the migration here).
• Default the doctype to the one from the project and save that to the config (we already do this on master).

Also, for the footer call, I reliaze we need to know the doctype because we want to generate the links to the other docs, as Eric mentioned, querying the config from the build objects could be slow. So, what about passing the doctype to the footer request (I mean saving it in the rtd-data.js file)? Old projects will still need the old logic.

Another idea, why not just send the whole ulr? and replace the lang/version in the api footer? Again, old projects will need to keep the old logic.

For the resolver, honestly I have no idea why do we need to use documentation type there 😞, some locig looks like the one from the footer...

/cc @rtfd/core

[humitos]

Also, for the footer call, I reliaze we need to know the doctype because we want to generate the links to the other docs, as Eric mentioned, querying the config from the build objects could be slow

How much is slow? We should need some measure here. I suppose that PostgreSQL is optimized to do queries inside a JSON field. In case we consider that it's slower than we want, we can always denormalize the db a little and duplicate this data into a normal field (which will perform fast).

[stsewd]

How much is slow? We should need some measure here. I suppose that PostgreSQL is optimized to do queries inside a JSON field.

Mostly it's because we are doing a query on a larger table (builds) on each request to the footer API, I'm not worried in other parts of the code (maybe in the resolver?). That's why I suggest saving this data on the rtd-data.js file, we alredy save a lot of info for the footer API there.

[agjohnson]

Caching could be fine here as well, we could set the cache on each build even. This would remove the need to have to update the Version object from a project build.

We could either treat this like a feature flag, or use a date based feature flag to change this UI. Old projects might know how to edit the project documentation type. New projects should be pushed to use the config.

[stsewd]

So, I've investigated the resolver, doctype is used inside https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/core/resolver.py#L93-L93

in https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/core/resolver.py#L254-L254

The usage of resolve_path are:

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/core/views/serve.py

Used to serve docs from /docs (doctype isn't always needed), and we don't used it in production, because we serve from nginx.

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/redirects/models.py

Used in redirects. Isn't used at all, or it shouldn't, because it only returns a redirect, isn't serving from a file.

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/projects/tasks.py#L1149-L1149

En tasks (_manage_imported_files). Isn't used at all, because all files are valid files (html and not-html)

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/projects/tasks.py#L1215-L1221

[ericholscher]

Used to serve docs from /docs (doctype isn't always needed), and we don't used it in production, because we serve from nginx.

This is not true for the .com or in local dev. I don't see where we'd need to know the file ending of a file tho, we should just pass it in explicitly.

[stsewd]

Looks like not, https://docs.djangoproject.com/en/1.11/ref/views/#django.views.static.serve it has show_indexes=False by default and nginx accepts an uri https://www.nginx.com/resources/wiki/start/topics/examples/x-accel/#x-accel-redirect

I'd say we are safe, I'll do some tests anyway.

[stsewd]

Other function where we use this is readthedocs.core.resolver.resolve

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/core/resolver.py#L162-L162

(we call resolve_path from here).

And that is used here:

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/projects/models.py#L24

I checked the code, and we don't need the doctype here

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/core/views/serve.py#L43-L43

Here there is a place where we are breaking something

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/core/views/serve.py#L122-L132

Here we are losing redirects like prject.rtd.io/page/subpage -> docs.rtd.io/en/latest/index.html#subpage

Which in my opinion doesn't look very relevant, and I don't think people are even aware of that...

And the last place

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/core/templatetags/core_tags.py#L39-L43

This is used in search, so not really sure if this will break something

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/templates/projects/project_analytics.html#L67-L67

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/templates/search/elastic_project_search.html#L50-L50

https://github.com/rtfd/readthedocs.org/blob/2bf6e9943c4177d11e125fd077a07f7168fce044/readthedocs/templates/search/elastic_search.html#L144-L144

But at first sight I would say that we always index the name of the file as a full path, so it shouldn't be any problem.

[willingc]

I believe that we ran into the redirect issue re: htmldir builder in the latest CPython devguide build. python/devguide#483

[willingc]

FYI. Sphinx builder dirhtml in Sphinx 2.0+ uses the name dirhtml. I think RTD v2 config is using htmldir. I'm not sure if the two are mapping correctly.

[stsewd]

Just an update here, looks like we depend on the doctype on many places. I think we could get rid of it, but it may be a big refactor. So, I think moving the doctype to the version model is a better solution for now.