readthedocs.org icon indicating copy to clipboard operation
readthedocs.org copied to clipboard

Migrate user-facing feature flags into the product

Open humitos opened this issue 2 years ago • 3 comments

I'm creating an issue from an old Trello card that we have, so we can prioritize this work easily.

Taking a look at https://docs.readthedocs.io/en/stable/feature-flags.html#feature-flags, there are still some feature flags that we can migrate/remove:

  • PIP_ALWAYS_UPGRADE: this can be replaced by build.jobs.post_create_environment: pip install -U pip
  • UPDATE_CONDA_STARTUP: replaced by build.jobs.pre_create_environment: conda install conda
  • CONDA_APPEND_CORE_REQUIREMENTS: I'm not sure there is an easy way to replace this, but we could suggest a bashy solution maybe?
  • DONT_OVERWRITE_SPHINX_CONTEXT: this one does not seem to be replaceable either
  • LIST_PACKAGES_INSTALLED_ENV: this can be done now via build.jobs.pre_build: pip freeze

I think we could add these examples to FAQs (or even the "Build customization" page) and remove them from the page linked.

humitos avatar Jun 02 '22 13:06 humitos

UPDATE_CONDA_STARTUP: replaced by build.jobs.pre_create_environment: conda install conda

Note that this is causing issues like https://github.com/readthedocs/readthedocs.org/issues/9306

LIST_PACKAGES_INSTALLED_ENV: this can be done now via build.jobs.pre_build: pip freeze

For internal usage, we are now saving this into BuildData objects.

humitos avatar Jun 08 '22 09:06 humitos

I'm re-opening this issue because we want to migrate users off from them. This could include sending them an email with the instructions and date or opening PRs on their repositories.

humitos avatar Jun 30 '22 18:06 humitos

Yep, we have some good conversation about this here: https://github.com/readthedocs/readthedocs.org/pull/9377

ericholscher avatar Jun 30 '22 23:06 ericholscher

We've done a lot of work on #9779 to remove old/deprecate feature flags. We still have some of them in our code base, but they are there for different reasons.

Exposed to the users

  • CONDA_APPEND_CORE_REQUIREMENTS: this one still seems to be useful. We will be able to remove it once we don't install dependencies on behalf of the user (mock, pillow, recommonmark, mkdocs, readthedocs-sphinx-ext, sphinx, sphinx_rtd_theme). I'm fine keeping it for now until we stop installing these. This feature flag cloud be migrated to #9698 if we want to do something about it.
  • DONT_CREATE_INDEX: this is exposed because some people may be using a static index.html page and they may require this feature flag (we have only 1 project using this flag). My position here is to remove this feature flag completely and also remove the logic to create an index.html page automatically. The instructions exposed on that page are not great (see https://github.com/readthedocs/readthedocs.org/blob/6ad9401329a6dd80aac34307c9908cd4600dab69/readthedocs/doc_builder/base.py#L81-L97) and I think we can solve it better in a different way (e.g. in the 404 handler we can detect if it's the index.html from the root URL and expose more guidance there --point to the tutorial maybe?, or simple just don't show anything). This is aligned with our philosophy that avoids doing things on behalf of the users in the build process.

Require migration path

There is a nice table at https://github.com/readthedocs/readthedocs.org/issues/9779#issuecomment-1587547386 that we can use to continue the conversation of these feature flag that require some work together with the users (e.g. onsite/email notifications, open PRs on their repositories, etc)

humitos avatar Jun 22 '23 10:06 humitos

Yea, I'd like to get rid of the index.html generation. It's definitely from an older time of the platform, and the more we can simplify our builds the better. We should just raise a warning to the user in a notification if they don't generate an index.html on build... or just fail the build?

ericholscher avatar Jun 22 '23 17:06 ericholscher

Yeah. Failing the build is the easiest and also it's going to be pretty clear to users what the problem is. I doubt there are valid projects serving proper documentation without an index.html file. So, I'm fine failing the build. I'll open a PR about this.

humitos avatar Jun 26 '23 09:06 humitos

Is this fully done, or did it get auto-closed? 🤔

ericholscher avatar Jun 28 '23 22:06 ericholscher

I think this is done already. We are using #9779 to track the deprecation emails and more. I think it's fine to close this issue at this point.

humitos avatar Jun 29 '23 09:06 humitos