Using RTD's "Version warning" feature

[amotl]

@amotl maybe a bit late to the party, but can't #322 be resolved via https://docs.readthedocs.io/en/stable/versions.html#version-warning ?

Originally posted by @msbt in #322 (comment)

[amotl]

Dear @msbt,

thanks for your suggestion. I just checked the projects [1,2] and found the corresponding feature to be enabled already.

Now, we might want to investigate why it's not working. On this matter, I discovered this output on the Browser console. Maybe it is the culprit?

With kind regards,
Andreas.

[1] https://readthedocs.org/dashboard/crate/advanced/
[2] https://readthedocs.org/dashboard/crash/advanced/

[msbt]

@amotl I don't know to be honest, those errors are appearing since we got rid of our custom docutils (!) doctools and used the default one.

[amotl]

Hm. I can barely remember it and wonder how it could relate to docutils in any way. Can you provide a reference (commit/PR) which you think was responsible?

Edit: Ah, you meant to say doctools. I see. Do you think it was related to #249 or #255?

[msbt]

I think it was around that time here or rather this one when you/we added all the webpack stuff. Before that we had a custom doctools.js that shipped with the theme I believe

[msbt]

Hah, beat me to it, yes, at least I think that was the time when the error showed up

[amotl]

I can see doctools.js being referenced, but I can't find it back. Neither the file, nor the commit which removes it.

[msbt]

Hrm odd, maybe I'm remembering it wrong, but I'm pretty sure it was around that time. I'll check back on Monday if I can find something more concrete, sorry for the wild goosechase :/

[msbt]

@amotl ok maybe that wasn't it, but we do have one repo with an old template online, I'll compare a current one with https://crate.io/docs/crate/reference/en/3.3/

[jayeff]

@msbt @amotl I opened a PR which would upgrade docs-theme for crate 3.3 branch to latest version: crate/crate#12209 Preview is here: https://crate--12209.org.readthedocs.build/en/12209/

Just upgrading docs-theme alone does not seem to fix the version-warning though

[amotl]

Hi again,

I just looked up the corresponding implementation by RTD and wanted to share corresponding pointers. They can be useful to find out why it doesn't work on our pages.

• Version comparison and evaluation of the is_highest flag happens within views/footer_views.py.
• The banner is rendered by js/doc-embed/version-compare.js.
• In the footer, other details about versioned documenation is handled, but nothing about the version warning / is_highest flag. See js/doc-embed/footer.js.

I hope this will help to give us a trace to follow.

With kind regards,
Andreas.

[amotl]

Hi again,

@msbt found that it works perfectly on the readthedocs.io domain. The RTD-native version warning admonition box can be inspected on https://crate.readthedocs.io/en/4.6/ and looks like

I think this is sweet. @seut: If you don't have any objections, I would prefer using this solution instead of our own thing #322. What do you think about it? ¹

With kind regards,
Andreas.

Footnotes

1. @msbt continues to trace the problem outlined at https://github.com/crate/crate-docs-theme/issues/323#issuecomment-1064193967, which currently happens to render all RTD injections defunct on the crate.io domain, blocking this feature. He told me that he might already have a clue why that happens. ↩

[seut]

Great! Yeah definitely prefer this as well. Thanks a lot for investigating @amotl @msbt

[amotl]

Hi again,

@msbt shared some insights about his investigations with me, thanks a stack.

• At [1], we can see the variable proxied_api_host is responsible for designating the right URI to RTD's API endpoint. By default, it is configured as /_, see [2]. This will have to be adjusted appropriately on a per-project level.
• At the same time, we will have to take care that the Nginx reverse proxy configuration will pass through those requests correctly. Currently, this is happening at crateio_docs/rewrites/02_versions.conf#L48-L49.

With kind regards,
Andreas.

[1] https://github.com/readthedocs/readthedocs.org/blob/7.4.1/readthedocs/core/static-src/core/js/doc-embed/footer.js#L67
[2] https://test-builds-webhook.readthedocs.io/en/latest/conf.html

[amotl]

Both readthedocs/readthedocs-sphinx-ext#87 and readthedocs/readthedocs-sphinx-ext#88 reference the proxied_api_host variable.

Code which references proxied_api_host and proxied_api_url can be found at ¹ and ².

Footnotes

1. https://github.com/readthedocs/readthedocs-sphinx-ext/blob/2.1.5/readthedocs_ext/readthedocs.py#L172-L173 ↩

2. https://github.com/readthedocs/readthedocs.org/blob/7.4.2/readthedocs/projects/models.py#L612-L634 ↩

[amotl]

A full _/api call, for getting the footer RTD HTML inject, looks like

https://crate.readthedocs.io/_/api/v2/footer_html/?callback=callback&project=crate&version=latest&page=index&theme=crate&format=jsonp&docroot=/docs/&source_suffix=.rst

All _/api URLs must at least satisfy, for example, this URL scheme, in order to deliver the corresponding RTD HTML inject code.

https://crate.readthedocs.io/_/api/v2/footer_html/?project=crate&version=latest

All other variants, like omitting the trailing slash, or omitting one of the required parameters, will make the API respond with 404 Not Found. So, the rewriting applied by Nginx must exactly match/reproduce this URL pattern.

[amotl]

Hi again,

I hope that two patches will help to improve the situation.

Improve Nginx configuration

The first patch, https://github.com/crate/infrastructure/pull/2850, relates to appropriate Nginx configuration in order to forward query string parameters to an upstream HTTP server using proxy_pass ¹.

-    proxy_pass https://example.readthedocs.io:443/$1;
+    proxy_pass https://example.readthedocs.io:443/$1$is_args$args;

Reference/demonstration projects/URLs:

• RTD inject asset for crate-tutorials (does not work yet)
• RTD inject asset for crate-howtos (works, after adjusting the Nginx configuration manually on this upstream configuration snippet)

Improve Sphinx/RTD project configuration

The second patch, coming from #326 and #328, attempts to adjust RTD's API URL ² proxied_api_host context variable on a per-project level, to make the project use an API URL like outlined above for crate-howtos ³, essentially prefixing it with docs/crate/howtos, coming from the html_baseurl context variable of each individual project.

This code has not been verified yet. Pretending that it will work, let's follow our established procedure of making a patch release and testing it on a single project before rolling it out on all the other projects.

Credits

Thanks again to @msbt for investigating and educating about the background of this issue.

With kind regards,
Andreas.

Footnotes

1. https://stackoverflow.com/a/8130872 ↩

2. https://docs.readthedocs.io/en/stable/api/v2.html#undocumented-resources-and-endpoints ↩

3. https://crate.io/docs/crate/howtos/_/api/v2/footer_html/?project=crate-howtos&version=latest ↩

[amotl]

Hi again,

the improvements above have been verified on behalf of the crash project. There are three indicators where we can see it works now.

Indicator I

You can see that it works by visiting the most recent documentation version at https://crate.io/docs/crate/crash/en/0.27/, where the RTD-native version chooser is now active at the bottom right corner. We probably will want to hide this again at the JavaScript or CSS level. I had already discussed this detail with @msbt.

Indicator II

On the other two documentation versions, after rebuilding them, the RTD-native version warning admonition box is now displayed correctly.

• https://crate.io/docs/crate/crash/en/0.25/
• https://crate.io/docs/crate/crash/en/0.26/

Indicator III

At https://readthedocs.org/dashboard/crash/traffic-analytics/, we can see that the RTD-native analytics tracking also started working.

With kind regards,
Andreas.

[msbt]

Nice one @amotl, looking good! Now we need to figure out how to block/hide that injected readthedocs box at the bottom right, because all the links it contains are on rtd and not crate.iio (i.e. https://crash.readthedocs.io/en/latest/ and https://crash.readthedocs.io/en/0.27/).

Mika started a discussion about that a while back but I don't think this is possible out of the box just yet. It's a shame though, because with that box also come quite a few assets (including fontawesome) which waste traffic if we don't use them.

[amotl]

looking good

I like it as well.

Now we need to figure out how to block/hide that injected readthedocs box at the bottom right.

Exactly.

@autophagy started a discussion about that [the other day].

Thank you for refering to this discussion. I think the comment by @humitos at readthedocs/readthedocs.org#3233 (comment) gives some hints how to hide that element again:

Read the Docs injects the HTML returned by the /footer_html API call into div.rst-other-versions. If you remove that element from your theme, RTD won't be able to inject it.

It's a shame though, because with that box also come quite a few assets (including fontawesome) which waste traffic if we don't use them.

I see. Thanks for outlining this. We might want to take care within one of the next iterations.

[msbt]

Read the Docs injects the HTML returned by the /footer_html API call into div.rst-other-versions. If you remove that element from your theme, RTD won't be able to inject it.

That's the thing, I don't think this is true (anymore). It creates a top-level div.injected before the </body> tag, not much we could remove template-wise to avoid this.

[autophagy]

The way I got around disabled this in my own themes is by adding a CSS rule of:

.injected {
    display: none;
}

since the injected RTD html is wrapped in a div with class injected. Unfortunately, like @msbt said, I don't think this approach prevents resources being pulled over the network for it, but that's the best I could find.

[amotl]

Dear @autophagy,

thanks for your suggestion, @msbt implemented this with 8a46277.

Other than this, I also closed #322 because the patches outlined within this discussion enabled us to use the RTD-native version admonition box, see #323 (comment). Following that, I am also closing this issue.

Thanks to everyone who participated.

With kind regards,
Andreas.