🤖 Set build config via Sphinx ext #2360

webknjaz · 2025-01-17T03:44:58Z

This is an initial change allowing for restructuring how the config settings are computed and putting that logic into a dedicated Sphinx extension. The idea is that this extension may have multiple callbacks that set configuration for Sphinx based on tags and the environment state.

As an example, this in-tree extension implements setting the is_eol variable in Jinja2 context very early in Sphinx life cycle. It does this based on inspecting the state of current Git checkout as well as reading a config file listing EOL and supported versions of ansible-core.

The configuration format is TOML as it's gained a lot of the ecosystem adoption over the past years and its parser made its way into the standard library of Python, while PyYAML remains a third-party dependency.

Supersedes #2251.

This is an initial change allowing for restructuring how the config settings are computed and putting that logic into a dedicated Sphinx extension. The idea is that this extension may have multiple callbacks that set configuration for Sphinx based on tags and the environment state. As an example, this in-tree extension implements setting the `is_eol` variable in Jinja2 context very early in Sphinx life cycle. It does this based on inspecting the state of current Git checkout as well as reading a config file listing EOL and supported versions of `ansible-core`. The configuration format is TOML as it's gained a lot of the ecosystem adoption over the past years and its parser made its way into the standard library of Python, while PyYAML remains a third-party dependency. Supersedes ansible#2251.

docs/docsite/_ext/build_context.py

webknjaz · 2025-01-17T15:34:00Z

docs/docsite/_ext/build_context.py

+    def from_dict(cls, raw_dist: dict[str, list[str]]) -> 'Distribution':
+        return cls(
+            **{
+                kind.replace('-', '_'): versions


The dataclass has to have snake_case attrs so this is doing that conversion. It effectively maps end-of-life in the TOML config to end_of_life.

docs/docsite/end_of_life.toml

webknjaz · 2025-01-17T15:36:56Z

docs/docsite/end_of_life.toml

+[distribution.ansible-core]
+end-of-life = [
+  'stable-2.15',
+  'stable-2.14',
+  'stable-2.13',
+]
+supported = [
+  'devel',
+  'stable-2.18',
+  'stable-2.17',
+  'stable-2.16',
+]


@oraNod things here are the same as in the other mapping. Did you expect them to differ at some point? What's the semantic meaning you were going for?

Hey @webknjaz Sorry for the slow response. I've been focused on prep work for cfgmgmtcamp. My thinking was to use the "supported" list as a way to detect when the core repo creates a new stable branch. We could then get an announcement in matrix that it's time to cut a new stable branch for docs.

Here's a link to my comment where I mentioned this: #1695 (comment)

It was just an idea. And since it's not strictly needed as part of this PR maybe we should do it separately, if at all.

maybe we should do it separately, if at all

@oraNod yes, it's a good idea to exclude things that are out of the scope right now. However, it's also good to think about the desired data structure overall so it could be designed taking that into account right away.

I'm also confused about this ansible vs. ansible-core thing — both list the same list of EOL entries. What's that about? Did you anticipate these having different values under some circumstances? Any more info?

@webknjaz Makes sense to narrow the scope to only what is necessary.

For ansible vs. ansible-core there is a period during the release lifecycle where the core version reaches EOL before the Ansible package version. @samccann probably knows those details the best but we do need to mark both of those distributions as EOL at different times.

@oraNod but ansible has a different versioning scheme. Why does it list the ansible-core branches?

I don't fully understand the mapping to ansible then.

OK, maybe there is also a disconnect in my understanding. I'll try but my explanation could be off so bear with me. It would probably be easier to discuss on a call. That will have to be next week, I'm afraid.

Take the stable-2.18 branch. In the ansible/ansible repo this corresponds to version of 2.18 for core. The ansible/ansible-documentation repo also has a stable-2.18 branch that maps to 2.18 for the core docs build and 11 for the package docs build. It is one branch that maps to two builds that EOL at different times. (I feel like I'm explaining something you already know here. I don't mean to be patronizing.)

Would it be worthwhile to comment the toml so it's clear which branch names map to which ansible versions?

I don't think that is documented anywhere and is kind of tribal knowledge. Part of my goal was indeed to have a central, declarative place where we could clearly see which doc builds are EOL and which are still active.

@webknjaz Hey, so revisiting this after the chat yesterday.

I wonder if it would be better to use the VERSION constant instead of the branch name?

For example on the stable-2.18 branch we have this:

ansible-documentation/docs/docsite/rst/conf.py

Line 45 in 95095b6

VERSION = (

You can see the version for the package and the version for core more clearly.

I know the version remains devel until right up until release time. That shouldn't be too relevant for the EOL use case though. However it's worth pointing out that VERSION doesn't change when the branch gets cut.

I guess this doesn't address my other comment about the branch names to package version mappings. I think we can just put that into the maintainers file somewhere rather than shoehorning it into the toml.

Also, FYI, we discussed the missing version on docs.ansible.com. I forgot about this issue: #2211

I wonder if it would be better to use the VERSION constant instead of the branch name?

Perhaps. Depending on what we need to have in the config vs. in the command invocation. I already forgot half of that context :) We may need to hack on this together sometime...

gotmax23 · 2025-01-21T16:47:40Z

docs/docsite/_ext/build_context.py

+DOCSITE_EOL_CONFIG_PATH = DOCSITE_ROOT_DIR / 'end_of_life.toml'
+
+
+@dataclass(frozen=True)


What's the point of a frozen dataclass if it has mutable attributes?

It's just a nice default to have.

Also, the point of immutable data structures is that it's harder to get into a situation where their contents would be changed suddenly by functions where you pass them. Of course, this doesn't shield us from the mutable nested pieces of data, but it's the first step. It might be good to use some marshalling library in the future.

gotmax23 · 2025-01-22T19:32:11Z

docs/docsite/_ext/build_context.py

+from dataclasses import dataclass
+from functools import cache
+from pathlib import Path
+from tomllib import loads as parse_toml_string


Do we still need to support Python 3.10?

Could add an extra optional dependency if needed. Though given that 3.11 is hardcoded in the automation, I'd not bother.

The stable-2.13 is the only branch where we'd need to support Python 3.10. Initially I was thinking we'd want to merge this back to every branch for consistency. However it that requires extra dependencies for this extension I don't think it is worth it.

We can still declare the stable-2.13 branch in the toml file for completeness. And it would also be good to document the process to EOL docs in the README. We could just mention the "legacy" eol flag on those older branches. It's already set there anyway and there's no reason to ever revert it.

oraNod · 2025-02-13T18:11:28Z

docs/docsite/_ext/build_context.py

+from dataclasses import dataclass
+from functools import cache
+from pathlib import Path
+from tomllib import loads as parse_toml_string


The stable-2.13 is the only branch where we'd need to support Python 3.10. Initially I was thinking we'd want to merge this back to every branch for consistency. However it that requires extra dependencies for this extension I don't think it is worth it.

We can still declare the stable-2.13 branch in the toml file for completeness. And it would also be good to document the process to EOL docs in the README. We could just mention the "legacy" eol flag on those older branches. It's already set there anyway and there's no reason to ever revert it.

docs/docsite/rst/conf.py

oraNod · 2025-02-13T18:50:28Z

docs/docsite/end_of_life.toml

+[distribution.ansible-core]
+end-of-life = [
+  'stable-2.15',
+  'stable-2.14',
+  'stable-2.13',
+]
+supported = [
+  'devel',
+  'stable-2.18',
+  'stable-2.17',
+  'stable-2.16',
+]


@webknjaz Hey, so revisiting this after the chat yesterday.

I wonder if it would be better to use the VERSION constant instead of the branch name?

For example on the stable-2.18 branch we have this:

ansible-documentation/docs/docsite/rst/conf.py

Line 45 in 95095b6

VERSION = (

You can see the version for the package and the version for core more clearly.

I know the version remains devel until right up until release time. That shouldn't be too relevant for the EOL use case though. However it's worth pointing out that VERSION doesn't change when the branch gets cut.

I guess this doesn't address my other comment about the branch names to package version mappings. I think we can just put that into the maintainers file somewhere rather than shoehorning it into the toml.

oraNod · 2025-02-13T18:51:25Z

docs/docsite/end_of_life.toml

+[distribution.ansible-core]
+end-of-life = [
+  'stable-2.15',
+  'stable-2.14',
+  'stable-2.13',
+]
+supported = [
+  'devel',
+  'stable-2.18',
+  'stable-2.17',
+  'stable-2.16',
+]


Also, FYI, we discussed the missing version on docs.ansible.com. I forgot about this issue: #2211

gotmax23 · 2025-03-06T22:02:10Z

docs/docsite/_ext/build_context.py

Do you want to add this to

ansible-documentation/noxfile.py

Line 13 in c671fca

LINT_FILES: tuple[str, ...] = (

so the repo's Python linters (black, ruff, isort) are applied?

I despise black, so I'd rather not. Maybe, when somebody else takes over. I don't mind some of the others, though. I don't really use a full development environment with this repo, so whatever doesn't stand in the way of my workflow is good.

gotmax23 · 2025-03-06T22:03:38Z

docs/docsite/_ext/build_context.py

+
+@cache
+def _read_eol_data() -> EOLConfigType:
+    raw_config_dict = parse_toml_string(DOCSITE_EOL_CONFIG_PATH.read_text())


Not that the file is so large, but why not use the usual tomllib.load pattern instead of loading the whole text into memory first?

Ah, tomllib.load reads the whole thing into memory and passes it to tomllib.loads anyways (https://github.com/python/cpython/blob/0c088e44428d74d701fe1fc80a4cb4fe124c43f0/Lib/tomllib/_parser.py#L57).

Yeah, I also thought that it's probably not worth it.

docs/docsite/_ext/build_context.py

gotmax23 · 2025-03-06T22:04:50Z

docs/docsite/_ext/build_context.py

+        ) from proc_err
+
+
+def _set_global_j2_context(app, config):


Type annotation?

Thanks! This was meant as a demo, so I didn't pay attention at all the places, I suppose. Looks like the config type isn't imported. Will have to look it up later.

Suggested change

def _set_global_j2_context(app, config):

def _set_global_j2_context(app: Sphinx, config: object) -> None:

gotmax23 · 2025-03-06T22:06:10Z

docs/docsite/end_of_life.toml

+  'stable-2.17',
+  'stable-2.16',


The community package is only built for the latest ansible-core version. Should these be removed?

Suggested change

'stable-2.17',

'stable-2.16',

Dunno, I'm yet to fully understand the context. I'd leave this to Don. Or maybe we'll get to doing something together.

The community package is only built for the latest core version but the guideline for backporting and rebuilding docs is to maintain devel, the latest version, and the previous two versions.

In any case we should drop the "supported" portion of the toml since it is not used. I put that in my POC and it got copied over here.

The point was to have a declarative approach to the versions that are EOL and the versions that are actively maintained. I was also thinking of having a workflow that would periodically compare the list of "supported / active" versions and compare with the core branch and then notify the docs channel in Matrix when a new stable branch hits the core repo so we'd know to cut a new branch for docs.

Co-authored-by: Maxwell G <[email protected]>

oraNod

@webknjaz How about we drop the "supported" entries since we aren't using them at the moment?

The rest, barring a couple other review comments, seems good but maybe we should move away from using the branch names?

We have the version_helper for core that gets versions from https://github.com/ansible/ansible/blob/b3d21e3ad21a9ea731b5137a6a0e15c033c3a7d9/lib/ansible/release.py#L20

That is used as the single source of truth for the core version so maybe we should stick with it?

For the Ansible community package, the version gets passed in through the makefile with the ANSIBLE_VERSION environment variable.

I guess that complicates things in the extension but maybe it's a better approach from the maintainer perspective? What do you think? Cheers.

oraNod · 2025-03-27T15:32:09Z

docs/docsite/end_of_life.toml

+supported = [
+  'devel',
+  'stable-2.18',
+  'stable-2.17',
+  'stable-2.16',
+]


Suggested change

supported = [

'devel',

'stable-2.18',

'stable-2.17',

'stable-2.16',

]

oraNod · 2025-03-27T15:32:43Z

docs/docsite/end_of_life.toml

+supported = [
+  'devel',
+  'stable-2.18',
+  'stable-2.17',
+  'stable-2.16',
+]


Suggested change

supported = [

'devel',

'stable-2.18',

'stable-2.17',

'stable-2.16',

]

webknjaz · 2025-04-01T11:58:30Z

@webknjaz How about we drop the "supported" entries since we aren't using them at the moment?

Probably. I just don't want to remake the config structure multiple times. Do we still need that ansible vs. ansible-core separation in there?

The rest, barring a couple other review comments, seems good but maybe we should move away from using the branch names?

Sure, branches can be computed if needed.

We have the version_helper for core that gets versions from ansible/ansible@b3d21e3/lib/ansible/release.py#L20

That is used as the single source of truth for the core version so maybe we should stick with it?

For the Ansible community package, the version gets passed in through the makefile with the ANSIBLE_VERSION environment variable.

I guess that complicates things in the extension but maybe it's a better approach from the maintainer perspective? What do you think? Cheers.

Ideally, the version should be sourced from packaging metadata.

Something like https://github.com/nedbat/scriv/pull/144/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R15-R16 would do.

oraNod · 2025-04-02T12:22:26Z

Probably. I just don't want to remake the config structure multiple times. Do we still need that ansible vs. ansible-core separation in there?

Yes, ansible vs ansible-core is necessary. Those versions go EOL at separate times.

Ideally, the version should be sourced from packaging metadata.

Something like https://github.com/nedbat/scriv/pull/144/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R15-R16 would do.

That makes sense but I'm not sure how to achieve that since packaging is done elsewhere outside this repo. An approach like that also seems like it would make things more complicated for maintainers; whereas the intention of my POC had in mind eliminating that tribal knowledge of which version of the Ansible community distribution corresponds to which version of the Ansible core distribution. That is not clearly documented or declared anywhere.

Maybe this will be the worst suggestion you'll hear all day but what if we added a simple pyproject.toml file to this repo? I'm envisioning something like:

# devel branch
[project]
name = "Ansible documentation"
community-version = "devel"
ansible-core-version = "devel"

# latest branch
[project]
name = "Ansible documentation"
community-version = "11"
ansible-core-version = "stable-2.18"

I know this is an abuse of the pyproject.toml. I've been reading through the versioning spec and various threads about monorepos. It doesn't look like there is a proper way to achieve what I have in mind according to PEP 621. Then again we aren't building distributions from this repo so don't really need to take that into account. Maybe it would be better to have a custom version.toml file than something that bastardizes a specification.

A version.toml file in the repo root would also be good for the build process. We could read the version from there instead of passing it as an input, which reduces human error and requires someone to know which community distro version corresponds to which branch, etc.

webknjaz requested a review from oraNod January 17, 2025 03:44

webknjaz temporarily deployed to github-bot January 17, 2025 03:45 — with GitHub Actions Inactive

webknjaz mentioned this pull request Jan 17, 2025

EOL sphinx extension #2251

Closed

webknjaz force-pushed the maintenance/extension-driven-build-config branch from 8ef3352 to ab0e151 Compare January 17, 2025 03:53

webknjaz temporarily deployed to github-bot January 17, 2025 03:53 — with GitHub Actions Inactive

oraNod added doc builds Relates to building the documentation tooling This PR affects tooling (CI, pr_labeler, noxfile, linters, etc.) but not the docs builds themselves. labels Jan 17, 2025

webknjaz commented Jan 17, 2025

View reviewed changes

docs/docsite/_ext/build_context.py Show resolved Hide resolved

webknjaz commented Jan 17, 2025

View reviewed changes

docs/docsite/end_of_life.toml Show resolved Hide resolved

webknjaz commented Jan 17, 2025

View reviewed changes

gotmax23 reviewed Jan 21, 2025

View reviewed changes

gotmax23 reviewed Jan 22, 2025

View reviewed changes

oraNod reviewed Feb 13, 2025

View reviewed changes

gotmax23 reviewed Mar 6, 2025

View reviewed changes

Annotate _get_current_git_branch() w/ return type

dd926a3

Co-authored-by: Maxwell G <[email protected]>

webknjaz temporarily deployed to github-bot March 7, 2025 23:23 — with GitHub Actions Inactive

webknjaz moved this to 🗒️ Backlog 📝 in 📅 Procrastinating in public Mar 7, 2025

webknjaz added this to 📅 Procrastinating in public Mar 7, 2025

oraNod reviewed Mar 27, 2025

View reviewed changes

		DOCSITE_EOL_CONFIG_PATH = DOCSITE_ROOT_DIR / 'end_of_life.toml'


		@dataclass(frozen=True)

	def _set_global_j2_context(app, config):
	def _set_global_j2_context(app: Sphinx, config: object) -> None:

🤖 Set build config via Sphinx ext #2360

Are you sure you want to change the base?

🤖 Set build config via Sphinx ext #2360

Conversation

webknjaz commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oraNod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz commented Apr 1, 2025

oraNod commented Apr 2, 2025

webknjaz Mar 7, 2025 •

edited

Loading