Moving readthedocs-build into RTD

[agjohnson]

Our original thoughts around this package were to create a CLI tool that could mimic what RTD does normally. Eventually, the entire build process would rely on this tool to execute. All of the configuration parsing would be in this package, as well as virtualenv management.

Needless to say, our goals were lofty here.

I think going back on the design decisions here and pushing the config parsing code back into RTD itself makes a lot of sense, because:

• We have to duplicate a lot of logic around configuration defaults, because this is a separate package without knowledge of the project tables
• The virtualenv management and cli portion of this library aren't useable currently
• Changing our config requires contributors to come over to this library first, then make a complementary patch on RTD. This is a lot of work and the fact that this is a separate library isn't a benefit to contributor onboarding

We could:

• Merge this into the .org
• Plan on writing things in a way that we could theoretically pull the code out if it ever became useful on it's own (my guess is probably not)
• Clean up our default configuration selection on the .org

[ericholscher]

I'm +1 on merging it back in. It will vastly simplify our YAML config updates going forward, and I think rearchitecting this with the new docker build images will lead to a much different solution to the original problem that this code intended to solve.

My primary reservation here is that integrating our build process more into the model-based structure that RTD uses. One of my goals with having this be separate was that it would force us to keep everything here defined in YAML and not have a bunch of state in the DB. Obviously that hasn't happened, but I'd still like to keep that design goal in mind when we work towards merging this back into the code over time.

My concern above relate to the overarching "how do we support YAML and the DB" design decision that is at the heart of a lot of this complexity. If we didn't have state in the DB, then merging these makes less sense I think.

[humitos]

I'm not familiarized with the problems of having this code in a separate package and by reading the description (and looking the problem from outside) I don't really feel the issues. I see them, but I haven't been there before...

• I liked the idea of having this in a separate module mostly because that idealistic goal of being able to just run this outside RTD, but that seems it's not going to happen.

• The flow of having to make two PR is familiar to me and makes sense when you have an outside library. It's a little more tedious in some cases but it's cleaner in my opinion.

• Duplicated code to handle the default sucks :) --and I'd say that this is the worst

... but,

Supposing that we don't rely on DB state, is still a good movement to merge this into the core?

What stops us to stop relying in state from the DB? I have two things in the top of my head here:

1. not all the admin UI configurations are moved to/allowed from the YAML file: I'd say that the solution for this is keep working on adding options/configuration to the YAML file and removing/freezing them from the admin.

(I'd like to show a message in the Project Admin saying: "Hey! Use the YAML file as much as you can, we are deprecating this" as soon as possible)

1. there are some settings that are project-wide instead of version/build-specific:

• how other services handle this? Travis, for example
• we shouldn't have project-wide settings that affect the build process (requirements.txt, py version, install with setup.py, etc)
• project-wide settings could be always retrieved from the default_branch (tags, description, etc)

Maybe I'm deviating the conversation, but seems that solving this problem could affect in the decision of merging this code or not. Anyway, we can elaborate/discuss more about YAML and DB in another issue.

[ericholscher]

I think we'll always need to rely on DB state, but we need to clearly define what DB state we depend on. I think that requires really knowing what is "Project" and "Version" state, because some is "global" for a project, and other parts can change for each build. Once we have the data in a YAML file, how do we define "global" state without using the database, since the YAML data could be conflicting.

I think that explains my worry about depending on DB state, without having proper long term planning in place to know what state is OK to depend on.

[agjohnson]

We will never get away from the database, so I think that is a point for merging these two repositories. Eventually, we can prune the admin UI, but things like project description, default branch, default version -- these will never live in the YAML file.

Also, the use case for running this as a separate CLI command I think are gone, but I do have some thoughts. I'll open a separate PR to discuss this portion of the codebase.

For the immediate future, until we have a complete YAML config, the database will need to be merged in for the options that we determine can be moved. Once we do have a feature complete YAML config, we can disable parts of the admin UI for new projects. We'll still need to merge in the settings for legacy projects though, so we still can't avoid merging the database.

I think there is less of an argument for keeping a clean abstract between the two repositories if we'll always rely on the database.

I think that explains my worry about depending on DB state, without having proper long term planning in place to know what state is OK to depend on.

The next step after deciding where all of this lives is to gather all of our build settings and create a roadmap outlining the priority we put on moving settings. Each field should be considered for:

• Is this setting part of global state? If so, it's not part of our yaml spec. For instance Project.default_branch is independent of all repo branches, therefore is global state. Same for Project.repo and really anything that we use for display in our UI. It would be bad UX to update our UI from yaml fields that will differ between branch to branch.
• What is the impact of the field? We can look at % of projects that don't use the default value/null on a field to increase priority.
• Do we have features that we want to build on the field that depend on altering the UI? Higher priority to these fields as we want to avoid altering the UI. Again, priority to high impact fields.

With priority sorted, we can bucket these changes into milestones and have that published somewhere.

We can settle this in a separate document later though, nothing that needs to happen now.

[agjohnson]

It sounds like we're on the same page, but are we agreed to plan our move of the config management portion of -build into RTD? We can deal with the CLI portion later.

[ericholscher]

I'm +1 on moving the config logic into RTD itself. It will make things a bit simpler, but I do think we could write almost the same code in a separate package, if we broke the CLI aspect and just wrote an "RTD build configuration tool" that lived in a third party repo.

I think the "move the code into the same repo" decision is a proxy decision for "make the code only work for RTD, and no CLI or anything else". I'm +1 on reducing the scope and complexity of the code by making it assume RTD integration, and I do like having fewer repos/PR's/branches to manage, especially if the code is already tightly coupled to another repo. However, I do think where the code lives is less the discussion than what the code is expected to support.

[stsewd]

This was already done