Specify version of yaml spec to use

[stsewd]

Decision

Add version selection to schema. Schema version selection only uses major version, minor version is too granular. Current version is 1.0, work to complete spec will be under 2.0

Work required:

• Add version selection to all schema versions of the config file
• Work out a pattern to add multiple version parsing to the config file. How do we share config options that don't change? How do we share config options that changed a little? Does class based schema validation work best?
• Make an implied version 1.0 of the config file schema. I'm not sure if we also need schema tests for version 1.0, but it might also make sense to define the spec well.

Original Question

I didn't find anything related to this on this repo and on the other, so I creating this issue here due readthedocs/readthedocs-build#46.

I think would be nice if we allowed to specify the version of the spec used, so this way we can introduce breaking changes to the spec without to worry about previous projects to fail.

We can use semver perhaps?

And something like this on the yaml

Use specific version

version: 2.1
python:
    ...

Use version with latest minor patch

version: 2
python:
    ...

[humitos]

Thanks to raise this @stsewd! We chatted about this some weeks ago but we didn't opened an issue or write our thought anywhere, so this is a good start.

What would be the benefits of using a 2.x version (minor) in the file instead of using just 2 or 3?

[stsewd]

@humitos I was thinking something like the docker-compose.yaml spec https://docs.docker.com/compose/compose-file/. If is just introducing a new option or something "not breaking" we use a minor version, but if there is a big change on the spec, we use the mayor version, so this way we don't end with a version 13 too soon p:

[humitos]

Yeah, I also was thinking in something like docker-compose.yml :), but I think we can handle the semver (major and minor number) internally and only expose the major number to the users since I don't see any benefit on pinning to 2.1 instead of the latest 2.x. So, using just 2 should be enough.

Then, if we make breaking changes, that will be version 3; but we can still add features or fix some bugs under the 2 without user intervention.

What do you think?

[stsewd]

@humitos I was thinking to versioning the spec, not the implementation (so, in this case will be not bugs to fix, just modifications on the spec). I mean if the spec was something like this versions: None and then change to versions: [] we only change the minor versions (it's a breaking change but no too breaking). Not sure if that make sense 😁

[ericholscher]

+1 to versioning the config file. I think a major version is fine for now. If we add an optional setting to a version, can't we just read it if a user defines it? I don't know if we need a specific version number in order to support that.

The versioning is generally only useful if there contradictory definitions of a concept (eg. formats changing defaults), so that a user can explicitly define what they mean in that case.

[agjohnson]

+1 on major version, if only to reduce the number of specs we're supporting. We will have a lot more backwards incompatible changes to support here though, so I think a lot of changes will be bumping major versions anyways. New feature additions to the spec shouldn't break anyone's config.

[stsewd]

I thinking of using the strategy pattern to solve this. So, this way we can have a DocsBuilderV1 and DocsBuilderV2. Wich will be used after the clone/checkout step, this way we can have control over the submodules and the obviously the build steps.

Currently we:

• Start with the run method
• Run the run_setup method
  • setup_vcs -> sync_repo
    • checkout (actually we clone/update/update submodules and checkout here)
    • Hit the api to sync the versions and trigger new builds
  • We call to load_yaml_config to load the configuration file with the wrapper object
• We run the run_build method
  • Here we setup the build environment (python, conda, etc)
  • Finally, we call build_docs which build the html, search, single_html, pdf, epub

We need to modify this to use the pattern (or something like that p:)

• Start with the run method
• Run the run_setup method
  • setup_vcs -> sync_repo
    • Split the checkout method to:
      • get/update the repo
      • checkout
      • load the yaml and get the appropriated DocBuilder object (V1 or V2)
      • Use the DocBuilder object to decide what additional steps to take over the repo
    • Hit the api to sync the versions and trigger new builds (maybe we want to change/separate this in the future see Better UX when git checkout to a not existing branch/tag #4259)
• We run the run_build method
  • Here we let the DocBuilder to setup the build environment (python, conda, etc)
  • Finally, we let the DocBuilder build the html, search, single_html, pdf, epub

To get the DocBuilder object we need to parse the version key of the yaml (default to 1 if isn't present), pass the db data (through the version object) as a context to the BuildConfig object (we need one for each version), validate the schema. Expose an interface with do_additional_vcs_steps, setup_environment and build_docs (or maybe just one method for each build type; html, search, etc). We still need to see what objects and information is passed to this DocBuilder object (and the appropriate names 😃 ), but I think it may work :)

@agjohnson what do you think?

[agjohnson]

We can probably keep the build logic where it is, and just use conditional logic around the existing build code. The most important part is that we have a config object that knows what features it supports, and how to parse/validate those config values. Here's a psuedo code example:

https://gist.github.com/agjohnson/70a2b5e839bc8e7fd722f8eaf63d6676

[humitos]

I agree with Anthony. I wouldn't change to much the builder logic but make decision inside it considering the Config object.

Although, in the future, I'd like to have the pattern that Anthony is proposing for the Config object as a general rule and make our UpdateDocsTask something like you has described but instead of having those many ifs inside each of the steps use a class-based pattern and easy to follow :)

[stsewd]

This was done in #4355, but the feature is not activated yet.