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

Use theme to set all requirements for Crate Sphinx projects #294

Closed
6 tasks
nomicode opened this issue Jun 1, 2021 · 8 comments
Closed
6 tasks

Use theme to set all requirements for Crate Sphinx projects #294

nomicode opened this issue Jun 1, 2021 · 8 comments

Comments

@nomicode
Copy link
Contributor

nomicode commented Jun 1, 2021

I am creating this issue to address the underlying cause of crate/crate-jdbc#342.

Summary of the problem

How dependency resolution is done at build time

  • When RTD tries to build a Sphinx project, it reads the requirements from docs/requirements.txt.

    See below for examples.

  • Every docs/requirements.txt file specifies the crate-docs-theme.

    Some (most?) docs/requirements.txt files also specify additional dependencies (for example, to pin a specific version of sphinx)

  • The crate-docs-theme package setup.py file includes an install_requires list that also specifies dependencies (for example, to pin a specific version of sphinx).

Degrees of freedom

  • The specifications in docs/requirements.txt are fixed to the HEAD commit of the branch you are trying to build. There is no way to alter this configuration without updating the HEAD commit.

    For main or master, this typically isn't an issue.

    For release branches, this will involve a cherry-pick backport. Sometimes, backporting a simple docs configuration change to an older release branch of a software project causes unrelated test failures for the main project code. This can happen when the new test environment that was created to test the PR pulled in newer dependencies that were used before. These newer dependencies may introduce breaking changes, regressions, and so on. This typically requires one of the core project maintainers to spend some time fixing a bunch of unrelated project code tests.

  • The specifications listed in setup.py vary across versions of the crate-docs-theme package.

    When Pip is trying to satisfy the two sets of requirements, the only option at build time is to cycle through older versions of the crate-docs-theme package until one is found that does not conflict with docs/requirements.txt.

Conclusion

  • Any time docs/requirements.txt lists a package already listed as a dependency of the crate-docs-theme package (via setup.py), you open yourself up to "surprise" builds where RTD selects an old version of the theme. This will result in one or more of the live documentation projects using an out-of-date design (breaking seamless integration with the rest of the website).

    See Stale header and footer for some documentation variants/versions crate-jdbc#342 for an example of this.

Review of all docs/requirements.txt files

crate

# packages for RTD to pick up (not used for local dev)
crate-docs-theme>=0.12.0
sphinx-csv-filter
Pygments>=2.7.4,<3

crate-tutorials

sphinx==3.5.4
docutils==0.16
crate-docs-theme

crate-howtos

crate-docs-theme

crate-clients-tools

crate-docs-theme

crash

crate-docs-theme

admin-ui

crate-docs-theme

crate-python

crate-docs-theme

crate-jdbc

# don't pin crate version numbers so the latest will always be pulled when you
# set up your environment from scratch

crate-docs-theme>=0.7

# packages for local dev

sphinx-autobuild==0.6.0

# the next section should mirror the RTD environment

alabaster>=0.7,<0.8,!=0.7.5
setuptools<41
sphinx==1.8.5

crate-dbal

crate-docs-theme

crate-pdo

crate-docs-theme

crate-npgsql

crate-docs-theme

cloud-tutorials

crate-docs-theme

cloud-howtos

crate-docs-theme

cloud-reference

crate-docs-theme

croud

# Standard packages needed for the docs
###############################################################################

crate-docs-theme


# Project specific packages
###############################################################################

# Pip is run from the root directory, so this installs the `croud` package
.
sphinx-argparse==0.2.5

sql-99

crate-docs-theme

crate-docs

sphinx==3.5.3
crate-docs-theme

crate-docs-theme

# Install the local crate-docs-theme package by specifying parent directory,
# because `pip` is run from there.
--editable=.
sphinx==3.5.4
docutils==0.16

Proposed solution

Premise:

  • The crate-docs-theme should specify all requirements in the setup.py file and the Sphinx project-specific docs/requirements.txt file should only list a single dependency: crate-docs-theme (not pinned to any version, so the latest is always used).

Some of the projects already do this (see above).

To complete this work, I propose that we:

  • Update all docs/requirements.txt files so they only specify crate-docs-theme as a dependency
    • Backport this change to active release branches as necessary

Projects needing updates as above:

The theme also needs updating:

  • crate-docs-theme

    However, the crate-docs-theme docs/requirements.txt file is an exception.

    The --editable=. essentially installs the crate-docs-theme package from the parent directory, so that you can test changes to theme by building the docs. This line should stay. The next two lines (sphinx and docutils) can probably be removed.

@nomicode
Copy link
Contributor Author

nomicode commented Jun 1, 2021

@amotl take a look and let me know what you think. I'm happy to create the PRs and do the work to fix this issue. but I also thought you might like to take a shot? (i.e., to share knowledge/experience)

@amotl
Copy link
Member

amotl commented Jun 1, 2021

The crate-docs-theme should specify all requirements in the setup.py file and the Sphinx project-specific docs/requirements.txt file should only list a single dependency: crate-docs-theme (not pinned to any version, so the latest is always used).

I second this, let's make [1] the single source of truth. Shall we explictly say sphinx>=3.5.4,5 in setup.py then in order to retain the flexibility to evaluate it on Sphinx 4?

[1] https://github.com/crate/crate-docs-theme/blob/main/setup.py

@amotl
Copy link
Member

amotl commented Jun 2, 2021

In this context, I would also like to reference crate/crate-docs#52.

@amotl
Copy link
Member

amotl commented Jun 2, 2021

  1. I created Improve version pinning #295, Adjust to upstream changes crate-tutorials#70 and Overhaul documentation infrastructure crate-jdbc#346 in order to work towards this goal.
  2. I believe the dependencies on crate and croud will not do any harm.
  3. Would leaving project-specific dependencies like sphinx-csv-filter for crate or sphinx-argparse for croud do any harm to this plan?

@nomicode
Copy link
Contributor Author

nomicode commented Jun 8, 2021

  1. sweet!! thanks so much
  2. which dependencies? (i.e., what file or files are you referencing?)
  3. yeah I think that's fine. should be no problem to leave those project-specific deps in there

@amotl
Copy link
Member

amotl commented Jun 17, 2021

  1. which dependencies? (i.e., what file or files are you referencing?)

Yeah, I actually meant those referenced at 3., apart from crate-docs-theme, at:

[2a] https://github.com/crate/crate/blob/master/docs/requirements.txt
[2b] https://github.com/crate/croud/blob/master/docs/requirements.txt

All set!

@amotl
Copy link
Member

amotl commented Jun 17, 2021

@norosa: Do you believe we can close this ticket or is there some more work to do which I might be missing?

@nomicode
Copy link
Contributor Author

@amotl thanks for all the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants