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

Deprecation: require projects define a configuration file #10342

Closed
agjohnson opened this issue May 23, 2023 · 4 comments
Closed

Deprecation: require projects define a configuration file #10342

agjohnson opened this issue May 23, 2023 · 4 comments
Labels
Improvement Minor improvement to code

Comments

@agjohnson
Copy link
Contributor

agjohnson commented May 23, 2023

Edit: if you arrived at this issue for any reason, make sure to read the blog post that contains all the important info

For some background here, we identified a number of issues that stem from our historical decision to provide a default configuration file and experience to projects that do not define a project configuration file. Because of this stance, we do have a number of projects that do not know about our configuration file and rely on these defaults. This applies to both new projects (for example projects using our tutorial template) and well aged projects (projects started using Sphinx 1.8.5 for example).

Deprecation of this pattern will require that all projects define a Read the Docs configuration file in order to build. In most cases, a basic configuration file can be suggested to the project.

The downstream effect of requiring a configuration file is:

  • We can drop support for old build images that are also used by builds without a configuration file.
  • We can start to drop support for Sphinx 1.8.5, as we won't automatically opt projects into this version.

We discussed why we could provide new defaults to projects, and the conclusion here was a little nuanced and opinionated in our favor. But at the center of this conversation was also that all other similar services require the user define a configuration file, so this doesn't need to be an expected pattern on our platform.

The order of operations here would be:

@agjohnson agjohnson added the Improvement Minor improvement to code label May 23, 2023
@agjohnson agjohnson pinned this issue May 23, 2023
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap May 23, 2023
humitos added a commit that referenced this issue May 30, 2023
We are installing Sphinx 1.x based on a feature flag.
There are around 900 projects still using 1.x, based on
https://ethicalads.metabaseapp.com/question/250-projects-using-sphinx-timeserie

Note this feature flag is enabled by default for new projects starting on Oct.
20, 2020

We may need to send them an email about this and informing them how to create a
`requirements.txt` file to pin their dependencies. I don't think that the
migration to a config file v2 would enforce them to pin Sphinx.

Related #10342
humitos added a commit that referenced this issue May 30, 2023
This is a continuation of the work done for

* #10351
* #10342
* #10348
* #10355

In that work we added notifications, warning messages and finally a feature flag
to start failing the builds without a configuration file or using v1.

This commit removes all the code for config file v1 and building without a
configuration file. Due to that, it also removes the code that allows users to
use the older Docker images (testing, latest, stable)

This work can be merged once we enable the feature flag to fail the builds for
all the projects and we are happy with the results.
@ericholscher ericholscher moved this from Planned to In progress in 📍Roadmap Jun 6, 2023
@steveschnepp
Copy link

steveschnepp commented Jun 14, 2023

Would it be possible to have a default for every configuration option. Ideally set to the recommended one?

Such as build.os & build.tools.python being set to the best1 supported version. This would enable you to rollout new version gradually, as you will be in the driver seat. And for projects that depend on a particular version, they can opt-in to it.

Therefore if I'm just fine with every default, I'd have a file with just version inside. (Even that could have a default, currently 2).

PS: Thanks for your service, it's awesome!
PPS: Requiring an explicit config file is indeed a good idea on many levels.

Footnotes

  1. Not necessarily the latest. But the one that you are the most comfortable with.

@humitos
Copy link
Member

humitos commented Jun 14, 2023

Hi @steveschnepp! Thanks for you comment and your suggestion here 👍🏼

Would it be possible to have a default for every configuration option. Ideally set to the recommended one?

Such as build.os & build.tools.python being set to the best supported version. This would enable you to rollout new version gradually, as you will be in the driver seat

This is exactly what we have tried and it's how it currently works. Right now, if there is no .readthedocs.yaml file we set every configuration option to a particular default (e.g. Python 3.7). The problem we have here is that it's impossible to maintain updated without breaking thousands of projects with any minor change of these defaults. Every time we tried to update the Python version or the OS version, we broke so many projects that we had to rollback. In the end, we weren't able to update those defaults and we arrived at the situation we are today: the defaults are pretty old and for some use-cases they are completely broken 😞

This is part of the background behind this decision. If you want to know more, you can read our blog post about this at https://blog.readthedocs.com/migrate-configuration-v2/. I hope this explanation helps.

PPS: Requiring an explicit config file is indeed a good idea on many levels.

We are true believers that "explicit is better than implicit" 😉

@steveschnepp
Copy link

steveschnepp commented Jun 14, 2023

Thanks!

Actually it was that blog post that led me to here, as I wanted to open a new issue about the defaults, but since I found this one, I simply added a comment. 😉

I understand that the current defaults are old. But let's simply say : those are the v1 defaults.

On the other hand, the v2 defaults should be evolving over time, and indeed they might break many projects.
Yet it's by design, therefore it shall be properly documented. Those projects can then opt-in to fixed versions if they need to.

Which is a part of why requiring to have an explicit config file is a good idea.

We are true believers that "explicit is better than implicit" 😉

Well, for configuration, I was in that camp also. But now, I'm much more in the other side.

As as a consumer, I just want to use the sanest defaults and not think too much about them, unless I really need to.

If everything is explicit, it needs some maintenance from my side, and I won't automatically benefit from any upgrades/enhancement on your side.

@ericholscher
Copy link
Member

This is getting tracked in our largest list of deprecation (#10587)

@github-project-automation github-project-automation bot moved this from In progress to Done in 📍Roadmap Aug 16, 2023
humitos added a commit that referenced this issue Sep 26, 2023
We are installing Sphinx 1.x based on a feature flag.
There are around 900 projects still using 1.x, based on
https://ethicalads.metabaseapp.com/question/250-projects-using-sphinx-timeserie

Note this feature flag is enabled by default for new projects starting on Oct.
20, 2020

We may need to send them an email about this and informing them how to create a
`requirements.txt` file to pin their dependencies. I don't think that the
migration to a config file v2 would enforce them to pin Sphinx.

Related #10342
humitos added a commit that referenced this issue Oct 23, 2023
…10367)

* Deprecation: remove code for config file v1 and default config file

This is a continuation of the work done for

* #10351
* #10342
* #10348
* #10355

In that work we added notifications, warning messages and finally a feature flag
to start failing the builds without a configuration file or using v1.

This commit removes all the code for config file v1 and building without a
configuration file. Due to that, it also removes the code that allows users to
use the older Docker images (testing, latest, stable)

This work can be merged once we enable the feature flag to fail the builds for
all the projects and we are happy with the results.

* Deprecation: update documentation of config file

* Deprecation: update config file to remove old keys

* Deprecation: update config file v2 docs to deprecate keys

- python.version
- python.system_packages

People should not be using these keys.

* Remove auto-install requirements file found on project

* Make the deprecated fields read-only as discussed

* Remove tasks to send emails about deprecations

* Lint & Tests

* Remove reference to v1 config file docs

* Normalize `get_build_config()`

* Update tests for config file

* Update tests for collectors

* Update tests to follow the new deprecations

* Remove invalid tests

* More updates related to the config files

* Update more tests

* Lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code
Projects
Archived in project
Development

No branches or pull requests

4 participants