implement multiple .readthedocs.yml files per repo

[ewdurbin]

An attempted implementation of #8234.

Opening as draft PR. If the approach is reasonable, I'm happy to push this across the line with tests and docs.

Also as is, I'm unable to get a clean test suite pass locally on main... so hoping CI will show me what else I missed.

[humitos]

Thanks for opening this PR 💯 . We had a long conversation about supporting this feature in #8811 and we didn't arrive at consensus at that time. We decided to de-prioritize this work because we weren't sure about what was the right direction. We decided to start with a Sphinx extension and check for adoption (https://github.com/readthedocs/sphinx-multiproject/) to decide whether or not re-prioritize this work.

I'll try to use your PR and energy to recover that conversation with the rest of the team and see if we can move forward with a simple and basic implementation at least.

[ewdurbin]

Thanks for helping clarify! Sorry I missed the longer issue 🙃. Feel free to close this PR if this isn't the direction ya'll want to go :)

[benjaoming]

@ewdurbin I like the approach, my thoughts were around maintaining reproduce-ability.

This approach will work well from the beginning and without future debt if the configuration path (build_config_file field) is saved for each Version and Build as well.

There will be a number of scenarios to cater for, some now and some later, here's a bit of a raw dump of things that I find to be actionable... I'll need to hear back if others think that this can make the approach work:

• Guaranteeing that this doesn't introduce new bugs: We leave all build_config_file fields as None for all projects that are not using this feature. Or rather: We don't fill in this field with .readthedocs.yaml by default, None is good as .yml or .yaml is disputed anyways. ✔️
• When a new build runs, it should pick the associated Version.build_config_file (if it exists). Postponed for later
• When a new build runs, it should archive it's build_config_file value Postponed for later
• If an associated Version does not have build_config_file specified, it should be picked from Project Postponed for later
  • If Project.build_config_file is defined
  • If the Version was created after Project.build_config_file_modified
• Maybe: If an associated Version does not have build_config_file specified and an eligible value on Project.build_config_file is found, it should be stored on that Version.
• I haven't thought about Builds that are re-run.. but I think they don't need to do anything else than to use the build_config_file that's archived on the build. But we could make it possible somehow to update Project.build_config_file and re-run the latest build.
• When the user changes Project.build_config_file, they should be asked if they want this change to update current versions. They need to be able to choose those versions through some kind of list/date/version pattern.. not sure what works well. Postponed for later
• Maybe: build_config_file should be visible in the build log
• Version.build_config_file should be visible somewhere on Versions Postponed for later

Edited with updates:

• And we need a Project.build_config_file_changed field to track when this field is updated. Removed with input from @stsewd. We can use the automatically maintained model HistoricalProject for this, no actions required.
• Updated to trim out items that can come in a v2

[ewdurbin]

Thanks for the comprehensive overview @benjaoming.

Checking in here to see if there's any appetite for seeing this across the line. We're actively interested in publishing multiple doc sets (built with multiple tools so the multi-sphinx plugin won't work :() for at PyPI and this is blocking that.

[ericholscher]

@ewdurbin Thanks for the ping. We definitely want to make sure we can get this working for y'all. I will bring it up in our roadmap planning call tomorrow. 👍

[ericholscher]

@ewdurbin We have talked about this, and have put it on our upcoming sprint. Can you share a bit more about the use case you have here? I looked through the PyPI issue, but didn't see the exact problem. Is this mainly around multiple mkdocs projects, or mkdocs & sphinx mixed?

[ewdurbin]

We currently have developer focused docs that are being published via read the docs to warehouse.pypa.io.

We intend to add end-user documentation and a blog hosted out of the same repo, each on their own domain. Reasoning for multiple disparate sets of docs being built out of pypi/warehouse is to allow us to deliver code, docs, and blog changes in a single merge, allowing them to be reviewed side by side.

As far as the mixture of doc building tools, we're just a little burnt out on sphinx and are trying to move away from it 😅

[ericholscher]

@ewdurbin Ha, no worries on Sphinx -- it's not the perfect tool for everything :) We're also using things like Pelican on RTD these days 🐦

Just trying to understand the use case more, and make sure we're building something that will fit it.

[ericholscher]

@stsewd @humitos Curious what y'all think about moving forward with this approach to this, but behind a feature flag? So basically, this field shows up on the project after we enable it on the project. That at least unblocks this for users, and gives us a point of contact for anyone who needs this feature, and get some initial data on how folks are using it?

If we need to change approach in the future, at least folks will know they opted into this option?

[stsewd]

@ericholscher maybe we want to continue the discussion at #8811?

[ericholscher]

We did a call here, and are 👍 on moving forward with this approach (#8811 (comment)).

[ewdurbin]

Nice! I'll put some time into fleshing this out with tests/docs later this week.

[ericholscher]

This feels reasonably close. I need to do a bit further of a review, but noted a few places that seemed good to clean up.

I can take over this PR if you don't want to put it over the finish line. We normally deploy on Tuesday's, so we could aim to get this out next week I think.

[ewdurbin]

I can take over this PR if you don't want to put it over the finish line. We normally deploy on Tuesday's, so we could aim to get this out next week I think.

@ericholscher I wouldn't be circling back to this until probably Friday afternoon, but I am 100% ok with you taking this over.

[ericholscher]

@ewdurbin Great. I'll see if someone on our side can get to it this week 👍

[benjaoming]

@ewdurbin I'm gonna have a look here and might be pushing changes to the PR today. LMK if we're overlapping, but I'm guessing your afternoon is happening after I've signed off 👍

[ewdurbin]

@benjaoming I'm well over 5 hours from when I'd be poking at this, so no overlap likely :)

[benjaoming]

A quick heads up about some recent pre-commit changes from #10105

Basically, just ignore all linting errors that are unrelated to changes, run git commit --no-verify and we'll get to this in other PRs. I'm not quite clear about the way forwards right now.. removing the improved linting seems bad, but it's a bit of a mountain to climb for this PR. In my naivity, I tried to quickly fix some of the linting errors in this PR, but that caused more linting errors to pop up 🙃

[benjaoming]

Nice work btw @ewdurbin 🎉

The changes are working and we can build with a custom configuration file path and fail when it doesn't work.

I would like to know if we should inject a pseudo-command that just does like "Using custom config file: path.yaml" after the checkout is completed.

[benjaoming]

Sorry about the force-push there, I am running a bit short on time and while thinking I had a quick-fix for the linting issues, I got stuck with a bunch of unclean changes after resetting and re-running pre-commit.

[benjaoming]

Storing the custom build file path is also working, so I have my worries about reproducing old builds settled :)

[benjaoming]

I've added a test project here: https://github.com/readthedocs/test-builds/tree/monorepo

It seems to work for project1, just sorting out why project2 is failing atm.

[benjaoming]

One note on the new test project: We should try out PR builds to see how the feedback works. I'm supposing that the fact that we're appending the project name will make it show up nicely.

I'm assuming that it's gonna work fine, and that this can be a post-deployment inspection.

[benjaoming]

Just finished running a local test on Read the Docs for Business and it works without modifications 👍 The only issue is that the manual import isn't exposed, but I think that's an entirely separate issue.

[benjaoming]

@ewdurbin just wanted to give a quick update: @humitos has been testing the PR and we hope to be ready soon :)

[humitos]

Looks good to me. I made some comments and suggestions, but I think nothing is blocking.

I want to say that I'm still worried/concerned about these changes because it touches a lot of core places that are not easy to test via our testing framework. I recommend to do as much QA locally as possible to avoid deploy surprises and/or downtime. This implies, among others, testing the monorepo branch from test-builds but also as many branch from the same repository as possible making sure that Conda, different Python versions, build.commands, etc work as expected.

[benjaoming]

Just building a bunch of branches from test-builds locally now... if that all works, I'll go ahead and merge this 👍

[benjaoming]

Very very niche issue

While testing, I discovered this annoying little "race condition" bug, but it's mostly relevant for testers and not so likely in real life. It's a variation of #9942

If you only have the .readthedocs.yaml file in a special (non-default) branch and you configure the custom path BEFORE a newly added project has finished it's first successful build, then a state of "no branches detected" occurs. This is probably because the Advanced Settings form overwrites the default branch with an empty select input (whatever custom branch was defined hasn't yet been made part of the eligible branches because the build hasn't finished).

The workaround is to leave the setting blank, create a successful build and then change the .readthedocs.yaml after that 🤯

Related: #9606

[benjaoming]

Actually, the above issue might occur in real life if someone is manually importing a couple of new projects quickly -- it seems there are a ways to make the checkout step break in the first build, and the only way to recover it is to have a functional default branch setup. So re-visiting Advanced Settings might be required in order to make the first build complete the checkout step.

Seeing that it's pretty hard for me to even clearly write down the issue, I think it might be worth noting in #9942 if this is something that you run into @ewdurbin in the future.

[benjaoming]

🎉

[benjaoming]

@stsewd @ericholscher I was advised by @humitos to try some different existing builds to see if they still work. I found a Conda environment conda-env-py3.7 that builds fine in production, but it doesn't work locally for me.

The build that works fine but fails locally: https://readthedocs.org/projects/test-builds/builds/20117124/

I'm not able to understand what could have gone wrong from the local log:

Local build log

Read the Docs build information
Build id: 64
Project: test-builds
Version: conda-env-py3.7
Commit: e174713a43ab9b4b265796827e892dcd783b6da0
Date: 2023-04-14T14:47:44.591957Z
State: finished
Success: False


[rtd-command-info] start-time: 2023-04-14T14:47:48.053566Z, end-time: 2023-04-14T14:47:49.377798Z, duration: 1, exit-code: 0
git clone --no-single-branch --depth 50 https://github.com/readthedocs/test-builds.git .
Cloning into '.'...

[rtd-command-info] start-time: 2023-04-14T14:47:49.630143Z, end-time: 2023-04-14T14:47:49.701089Z, duration: 0, exit-code: 0
git checkout --force origin/conda-env-py3.7
Note: switching to 'origin/conda-env-py3.7'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at e174713 Update config file to version: 2

[rtd-command-info] start-time: 2023-04-14T14:47:49.980697Z, end-time: 2023-04-14T14:47:50.050752Z, duration: 0, exit-code: 0
git clean -d -f -f


[rtd-command-info] start-time: 2023-04-14T14:47:51.508482Z, end-time: 2023-04-14T14:48:04.469659Z, duration: 12, exit-code: 1
asdf install python miniconda3-4.7.12
Downloading python-build...
Cloning into '/root/.asdf/plugins/python/pyenv'...
python-build miniconda3-4.7.12 /root/.asdf/installs/python/miniconda3-4.7.12
Downloading Miniconda3-4.7.12-Linux-x86_64.sh...
-> https://repo.anaconda.com/miniconda/Miniconda3-4.7.12-Linux-x86_64.sh
Installing Miniconda3-4.7.12-Linux-x86_64...

EnvironmentLocationNotFound: Not a conda environment: /usr/src/app/checkouts/readthedocs.org/user_builds/6c3e7968f428/test-builds/conda/conda-env-py3.7


BUILD FAILED (Ubuntu 22.04 using python-build 2.3.17-4-gb3c91b37)

Inspect or clean up the working tree at /tmp/python-build.20230414144754.100
Results logged to /tmp/python-build.20230414144754.100.log

Last 10 log lines:
  urllib3            pkgs/main/linux-64::urllib3-1.24.2-py37_0
  wheel              pkgs/main/linux-64::wheel-0.33.6-py37_0
  xz                 pkgs/main/linux-64::xz-5.2.4-h14c3975_4
  yaml               pkgs/main/linux-64::yaml-0.1.7-had09818_2
  zlib               pkgs/main/linux-64::zlib-1.2.11-h7b6447c_3


Preparing transaction: ...working... done
Executing transaction: ...working... done
installation finished.

What I tried:

• Pulled in latest main
• Rebuilt docker images
• Ensure I'm on the correct branch :)
• Migrations are okay
• No errors from Docker output - build_1 and celery_1 not giving errors
• Other builds that I tested worked fine, this is the only one I found to be troublesome

Could someone else look into this?

[humitos]

@benjaoming

Have you tested the branch conda-env-py3.7 locally using origin/main from the application instead this PR?

asdf install python miniconda3-4.7.12

It seems you are compiling conda on each build. You should take a look at

readthedocs.org/scripts/compile_version_upload_s3.sh

Lines 4 to 9 in 60c74d5

	# Script to compile a tool version and upload it to the cache.
	#
	# This script automates the process to build and upload a Python/Node/Rust/Go
	# version and upload it to S3 making it available for the builders. When a
	# pre-compiled version is available in the cache, builds are faster because they
	# don't have to donwload and compile the requested version.

to compile it once and re-use it on every build. I hope you knew about this and you are not re-compiling Python and all the other tools on each build.

[benjaoming]

@humitos that worked 👍

I wasn't aware that the tools had to be bootstrapped like that, no. The Development Install guide doesn't mention it: https://dev.readthedocs.io/en/latest/install.html - it'd be interesting with a list that can quickly bootstrap the most commonly used tools.

[benjaoming]

I'm unable to come up with new ways that this might fail, so I'm merging it 🤞

@ewdurbin I'm guessing you might be very busy with PyCon, and that's great ❤️ The changes are scheduled to be deployed tomorrow (I can get back with a final confirmation on that), so if you do get around to trying it out, please send me a note here or in #8234 - or open up a new issue if something is borked or sub-optimal 😇

[humitos]

@ewdurbin I'm guessing you might be very busy with PyCon, and that's great heart

I'll be at PyCon 😎 , so if you are around we can take a look at any issue you may have with this during the sprints 😉

[benjaoming]

There's a little public example of the new feature available here:

https://github.com/readthedocs-examples/example-monorepo