ansible-documentation icon indicating copy to clipboard operation
ansible-documentation copied to clipboard

Update some direct links to use intersphinx links

Open samccann opened this issue 1 year ago • 8 comments

The conversation in https://github.com/ansible/ansible-documentation/pull/289 highlighted a couple of examples where we should really be using intersphinx links.

Rather than hold up that PR while we discuss the options, use this issue to track:

  • the examples that need to change
  • the intersphinx links changes to conf.py to make it all work

samccann avatar Aug 21 '23 16:08 samccann

@webknjaz - I'd appreciate your help here in understanding a few things wrt intersphinx links.

This line points to a ref anchor that only exists in the package docs, so it was causing a CI failure in the docs build (CI is based on core docs right now): :ref:execution environments <getting_started_ee_index>

We took out the Ansible intersphinx links a bit back (both the local ones and the ones in the conf.py). I think maybe we should have left the conf.py intersphinx settings because that wasn't the memory problem the local copies were.

So if we added one back for Ansible devel, would that make it possible to change that ref to something like: :ref:ansible_devel:execution environments <getting_started_ee_index>

samccann avatar Aug 21 '23 16:08 samccann

We took out the Ansible intersphinx links a bit back (both the local ones and the ones in the conf.py).

What do you mean by the “local intersphinx links”? Do you mean cache for the objects.inv files? In general, they're bad to store in Git because they are binary and Git is optimized for text. Adding, deleting or updating binary files are harmful to the repo size.

AFAIR, those cache files were set up to be the only source of intersphinx data. The configuration syntax allows using them as fallbacks, relying mainly on the online resources.

Could you remind me what were the memory problems you're talking about?

So if we added one back for Ansible devel, would that make it possible to change that ref to something like: :ref:ansible_devel:execution environments <getting_started_ee_index>

This syntax is invalid. You probably mean this:

:ref:`execution environments <ansible_devel:getting_started_ee_index>`

That ansible_devel thing is a key in the intersphinx mapping in conf.py and narrows down where a target is being picked up from. If you don't set it, it'd loop over all of the object inventories configured. I'm not sure what the memory usage implications would be, but in theory, it'd make the lookup faster at least.

webknjaz avatar Aug 22 '23 06:08 webknjaz

Oh, interesting, there's a newer role syntax for this too — https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#role-external, so it'd be:

:external+ansible_devel:ref:`execution environments <getting_started_ee_index>`

webknjaz avatar Aug 22 '23 06:08 webknjaz

I also noticed that the current config has incorrect intersphinx mapping value structure. It's supposed to be a tuple of 2 elements, not 3. But the second one can be a nested tuple. See: https://github.com/ansible/ansible-documentation/blob/4ec04d6/docs/docsite/sphinx_conf/all_conf.py#L284C35-L284C68.

webknjaz avatar Aug 22 '23 06:08 webknjaz

There's probably a good way to find all these manual URLs but for now: https://github.com/ansible/ansible-documentation/pull/308 and https://github.com/ansible/ansible-documentation/pull/309 are two new ones added recently.

samccann avatar Aug 24 '23 17:08 samccann

Among other things, it'd be good to run Sphinx's built-in linkcheck builder in CI to catch any dead links. This will cover normal URLs too.

webknjaz avatar Aug 24 '23 19:08 webknjaz

PR would be welcome for that one(linkchecker). I did try ages back but had way more false positives than I could handle. Probably had something configured wrong.

samccann avatar Aug 24 '23 19:08 samccann

FWIW modern versions of Sphinx are more resilient to false positives.

webknjaz avatar Aug 24 '23 19:08 webknjaz