Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design doc: magical builder maintainability and builder expanding API #8215

Closed
wants to merge 2 commits into from

Conversation

humitos
Copy link
Member

@humitos humitos commented May 26, 2021

Design document to discuss how to make the "magical" builder maintainable over
time and also how to extend the builder process to support other doctools eventually.

Note that this document is not finished yet, but it's a good starting point to start receiving feedback and continue the discussion.

Rendered version: https://docs--8215.org.readthedocs.build/en/8215/development/design/builder-maintainability.html

Design document to discuss how to make the "magical" builder maintainable over
time and also how to extend the builder process to support other doctools eventually.
Comment on lines +17 to +18
Besides, it removes the needing of using ``build.builder: 2`` since the *new builder* will extend the "magical" builder behavior
and just override what's required.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I had this idea clear in my mind when I started writing this, but then I realized that I didn't have a good and clear way as a user to disable all the magic. This may be a good topic to cover in our meeting.

@agjohnson
Copy link
Contributor

We talked through this design document and some of these ideas today on a call. The main takeaway we had is that this isn't as straight forward as we were considering, but versioning our builder would be an incredibly useful first step to changing builder logic.

Goals

To distill down some higher level points:

  • Our goals with changing builder process are, in order of descending importance:
    1. Make builders more maintainable by core team
    2. Increase configuration of build process
    3. Allow for a build process without magic
    4. Enable arbitrary build tools with good UX

Versioned builders

We are discussing versioning builders as a first step to this work. This would allow us to make incremental, backwards incompatible changes to builder logic without negatively affecting users.

We poked holes in our plan for versioned builders and came to the following conclusions:

  • A versioned builder class can't be used in the application

    • The application code only runs one installed version
      • Users can't select the builder package version then
    • The application code shouldn't run untrusted code
      • We can't extend a pattern like this to fit arbitrary builder classes later (goal 4)
    • It doesn't really help to version the application builder that we have now
  • Versioned builder classes could be executed in the docker container environment

    • Workflow would be:
      • We get builder package version from config
      • Versioned builder is installed in environment first
      • Application logic calls through docker to execute builder class
        • Builder class executes command directly?
    • This pattern gets us really close to providing a wrapper cli script to execute all of the same build commands locally for users
    • Very neat idea, but this defeats the purpose of versioning our builders to assist in making large changes to our builders
      • We have to drastically alter the build process to acheive this.
  • A third option is to version our implementation outside packaging

    • Use a build.builder: 2.1 to denote builder in configuration
    • Our application logic is where we keep idea of versioned build classes, not packaging
      • Similar to Stripe API and client interaction
        • User specifies the version of the API to use
      • Hopefully not accomplished with lots of conditional logic
        • Probably accomplished with a lot of class inheritance instead
    • This does not yet address arbitrary builder classes (goal 4), but we view this as a later priority anyways

The plan in this design doc to create a magic Sphinx builder and a separate non-magic Sphinx builder still stands. Without versioning figured out, we don't know how this looks technically.

Changes to other steps

Using the Sphinx builder as an example, we thought through how we might alter the build steps. I believe we came to the following conclusions:

  • We likely want to keep a number of steps the same for the current Sphinx builder
    • Search indexing is going to be better using Sphinx data than it would HTML
    • Overriding things like html_context does provide a lot of benefit
      • Users/themes are using html_context, so this would be a hostile change
      • If we move to another data structure, we're probably still translating that to html_context anyways
      • html_context is not the issue, the extraneous logic in conf.py.tmpl is the larger issue

We did not list "remove magic" as a goal of this work, because a good deal of the magic that helps the build process also provides really good UX to a lot of users. "Remove magic" is also maybe misleading, because we can't make the RTD build process match the local build process without dropping features like our flyout, search, etc.

So, in this scenario, we are mostly keeping the current Sphinx builder code, instead of our plan to throw away all of this functionality in search of a build process that we don't control.

Priorities and potential next steps

The priority order for work likely looks something like this:

  • Figure out if versioned builders is possible
  • Freeze current builder in a versioned builder. Version Sphinx builder as we change/remove features, options, flags, etc.
  • We add all options for build.jobs, these override default Sphinx builder commands
  • We add a separate builder class to remove all magic, this is independently selectable via config
  • We figure out a pattern to allow for community contributions and good UX around arbitrary builders
    • The build pack concept for code executed inside the docker container

@humitos humitos changed the title Design doc: magical builder maintainability and builder expnding API Design doc: magical builder maintainability and builder expanding API Jun 29, 2021
@humitos humitos added the Needed: design decision A core team decision is required label Jun 29, 2021
@mcarans
Copy link

mcarans commented Feb 23, 2022

I admit I don't have knowledge of the architecture, but here are a couple of questions:

  1. Is it possible to infer from the lack of a new builder section in the .readthedocs.yml that the current builder must be used?
  2. If not, is it possible to make .readthedocs.yml specific to the current builder and use a new filename for future builders eg. .readthedocsv2.yml?

@humitos
Copy link
Member Author

humitos commented Jul 11, 2024

We are in a lot better situation now. We've removed a lot of the magic and we are migrating all our projects to addons (see readthedocs/website#308)

The only relevant part from this document is the ability from the community to "package a builder" --but we've discussed we don't want to implement something custom and, if we are going to in that direction, using buildpacks or support running GitHub actions in our builders would be the path we would follow somehow.

It's also worth noting the "magic" part of our build process, was migrated to a Sphinx extension in case people want to keep the old/magical behavior for some reason: https://github.com/readthedocs/sphinx-build-compatibility/

@humitos humitos closed this Jul 11, 2024
@stsewd stsewd deleted the humitos/design-doc-magical-builder branch January 23, 2025 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants