Build: remove `append_conf` _magic_ from MkDocs #11206

humitos · 2024-03-11T15:50:05Z

Delete all the magic around MkDocs YAML that processes the mkdocs.yml file and define readthedocs theme automatically based on a feature flag, MKDOCS_THEME_RTD.

This PR removes:

automatically defining readthedocs as theme when MKDOCS_THEME_RTD feature flag is defined.
processing mkdocs.yml to add internal JS and CSS (embed and analytics) files automatically

This is another step forward on removing all the magic we perform on behalf the users and being more explicit about how to configure each doctool.

Reference:

Remove old assets inclusion/generation at build process addons#72 (comment)

Closes #8529

Delete all the magic around MkDocs YAML that processes the `mkdocs.yml`. file and define `readthedocs` theme automatically based on a feature flag, `MKDOCS_THEME_RTD`. This PR removes: - automatically defining `readthedocs` as theme when `MKDOCS_THEME_RTD` feature flag is defined. - processing `mkdocs.yml` to add internal JS and CSS (embed and analytics) files automatically This is another step forward on removing all the magic we perform on behalf the users and being more explicit about how to configure each doctool. Reference: * readthedocs/addons#72 (comment) Closes #8529

Listen to `Project.post_save` signal and enable Addons if the project was created after a specific date, it's MkDocs and it's the first time the `AddonsConfig` is created.

readthedocs/projects/signals.py

Co-authored-by: Santos Gallegos <stsewd@proton.me>

stsewd · 2024-03-11T17:50:06Z

readthedocs/projects/signals.py

+    if (
+        project.pub_date
+        > datetime.datetime(2023, 4, 1, 0, 0, 0, tzinfo=datetime.timezone.utc)
+        and instance.documentation_type == MKDOCS
+    ):


Shouldn't this logic always run? We are removing the old injection logic completely in this PR, if an old project is build, they won't have the old injections nor the new addons.

We have only a few projects on commercial using this feature only, and we plan to contact them telling how to migrate their configuration. Basically, they just need to define theme.name: readthedocs in their mkdocs.yml file.

I used this date here as an example, but we should give them more time than 20 days 😄 to migrate. We will contact them via email and write a blog post about this as well before merging this PR.

I re-read your comment and I think you are right 👍🏼 . I updated this code to remove the datetime limitation.

ericholscher

This looks like a solid change, and a big step forward for Addons & our new build approach ❤️ Not 👍'ing yet until we make a full plan for rolling it out to unblock it.

Blog post announcing the depreaction of the `mkdocs.yml` manipulation done by Read the Docs at build time, following the plan described. Related: * readthedocs/addons#72 (comment) * readthedocs/readthedocs.org#11206 * readthedocs/readthedocs.org#8529

* Post: MkDocs YAML manipulation Blog post announcing the depreaction of the `mkdocs.yml` manipulation done by Read the Docs at build time, following the plan described. Related: * readthedocs/addons#72 (comment) * readthedocs/readthedocs.org#11206 * readthedocs/readthedocs.org#8529 * Use double quotes The other ones don't render properly. * Apply suggestions from code review Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com> --------- Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

humitos · 2024-03-26T10:30:15Z

Merge this PR on April 15th and deploy it after it.

humitos · 2024-04-15T09:34:56Z

This PR was reviewed but it doesn't have an approval yet. I re-checked the changes and it seems it's fine to be merged and deploy. Please, take a final look at it today since we need to merge and deploy it today/tomorrow as we published in our blog.

stsewd · 2024-04-15T16:20:39Z

readthedocs/doc_builder/backends/mkdocs.py

@@ -389,8 +178,3 @@ def yaml_load_safely(content):
    information loss.
    """
    return yaml.load(content, Loader=SafeLoader)


I think we can remove all code related to SafeLoader and just call safe_load now.

We can't yet because it's used to get the final doctype for the version at

readthedocs.org/readthedocs/doc_builder/backends/mkdocs.py

Lines 66 to 68 in 8440aab

config = yaml_load_safely(fh)

use_directory_urls = config.get("use_directory_urls", True)

return MKDOCS if use_directory_urls else MKDOCS_HTML

This code will be eventually removed since the doctype doesn't have too much sense anymore now that we support many doctools more. We should probably need to have a conversation about how to delete doctypes completely in a safe way.

yeah, but don't think you need the custom loader to just read that setting.

Hrm, but without the safeloader we will executing !!python syntax, which is why we added it, right?

No if you call safe_load()

The custom loader was added to don't set those values to None after reading them (we were using safe_load previously).

Cool. Good point! I will delete this chunk of code then 👍🏼

Related #11206 (comment)

Keboletsetse · 2024-03-23T08:56:18Z

readthedocs/config/tests/test_yaml_loader.py

-    yaml_dump_safely,
-    yaml_load_safely,
-)
+from readthedocs.doc_builder.backends.mkdocs import ProxyPythonName, yaml_load_safely


readthedocs/config/tests/test_yaml_loader.py

humitos requested a review from a team as a code owner March 11, 2024 15:50

humitos requested a review from stsewd March 11, 2024 15:50

auto-assign bot assigned humitos Mar 11, 2024

humitos requested review from ericholscher and removed request for stsewd March 11, 2024 15:50

humitos added 3 commits March 11, 2024 17:15

Enable Addons if project is MkDocs

e7cbf0e

Listen to `Project.post_save` signal and enable Addons if the project was created after a specific date, it's MkDocs and it's the first time the `AddonsConfig` is created.

Listen to Version instead

a72ca02

Typo

2260f26

stsewd reviewed Mar 11, 2024

View reviewed changes

readthedocs/projects/signals.py Outdated Show resolved Hide resolved

Update readthedocs/projects/signals.py

90cca93

Co-authored-by: Santos Gallegos <stsewd@proton.me>

stsewd reviewed Mar 11, 2024

View reviewed changes

humitos added the Status: blocked Issue is blocked on another issue label Mar 11, 2024

ericholscher reviewed Mar 11, 2024

View reviewed changes

humitos mentioned this pull request Mar 19, 2024

Post: MkDocs YAML manipulation readthedocs/website#277

Merged

humitos added 2 commits March 19, 2024 17:36

Remove datetime restriction

8d78e5c

Merge branch 'main' into humitos/remove-append-conf-mkdocs

8440aab

humitos mentioned this pull request Mar 21, 2024

Deprecations: list of all past and ongoing deprecations #10587

Open

Keboletsetse mentioned this pull request Mar 23, 2024

Shouldn't this logic always run? We are removing the old injection logic completely in this PR, if an old project is build, they won't have the old injections nor the new addons. ethereum-lists/chains#4625

Closed

humitos removed the Status: blocked Issue is blocked on another issue label Apr 15, 2024

humitos requested review from ericholscher and stsewd April 15, 2024 09:33

Merge branch 'main' into humitos/remove-append-conf-mkdocs

6811075

stsewd approved these changes Apr 15, 2024

View reviewed changes

humitos merged commit 1c0f1c7 into main Apr 16, 2024
7 checks passed

humitos deleted the humitos/remove-append-conf-mkdocs branch April 16, 2024 08:32

humitos added a commit that referenced this pull request Apr 17, 2024

Cleanup: delete yaml_load_safely

ad9a2aa

Related #11206 (comment)

humitos mentioned this pull request Apr 17, 2024

Cleanup: delete yaml_load_safely #11285

Merged

humitos added a commit that referenced this pull request Apr 17, 2024

Cleanup: delete yaml_load_safely (#11285)

791643f

Related #11206 (comment)

This was referenced Apr 22, 2024

MkDocs 1.5 and the !relative YAML tag #10803

Closed

Remove old assets inclusion/generation at build process readthedocs/addons#72

Open

Keboletsetse reviewed Jul 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build: remove `append_conf` _magic_ from MkDocs #11206

Build: remove `append_conf` _magic_ from MkDocs #11206

humitos commented Mar 11, 2024 •

edited

Loading

stsewd Mar 11, 2024

humitos Mar 11, 2024

humitos Mar 19, 2024

ericholscher left a comment

humitos commented Mar 26, 2024

humitos commented Apr 15, 2024

stsewd Apr 15, 2024

humitos Apr 16, 2024

stsewd Apr 16, 2024

humitos Apr 16, 2024

stsewd Apr 16, 2024

stsewd Apr 16, 2024

humitos Apr 16, 2024

Keboletsetse Mar 23, 2024

	config = yaml_load_safely(fh)
	use_directory_urls = config.get("use_directory_urls", True)
	return MKDOCS if use_directory_urls else MKDOCS_HTML

Build: remove append_conf _magic_ from MkDocs #11206

Build: remove append_conf _magic_ from MkDocs #11206

Conversation

humitos commented Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericholscher left a comment

Choose a reason for hiding this comment

humitos commented Mar 26, 2024

humitos commented Apr 15, 2024

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

Build: remove `append_conf` _magic_ from MkDocs #11206

Build: remove `append_conf` _magic_ from MkDocs #11206

humitos commented Mar 11, 2024 •

edited

Loading