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

install sphinx test dependencies in all CI testing jobs #1772

Merged
merged 6 commits into from
Apr 19, 2024

Conversation

drammock
Copy link
Collaborator

should fix CI failures appearing in e.g. #1770

@drammock drammock changed the title CI: install sphinx test deps always install sphinx test dependencies in all CI testing jobs Apr 17, 2024
@drammock drammock added kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code labels Apr 17, 2024
@drammock
Copy link
Collaborator Author

hmm, all the pytest jobs pass now, but the build-site jobs all fail with:

Theme error:
setting ablog.inject_templates_after_theme occurs in none of the searched theme configs

not sure if it's related or coincidence, will look tommorow if possible unless someone else has time to look first.

@Carreau
Copy link
Collaborator

Carreau commented Apr 18, 2024

See sunpy/ablog#277 for the error message.

@trallard
Copy link
Collaborator

Right so for now we might want to pin our Sphinx version until there is a fix in Ablog for >= 7.3

@trallard
Copy link
Collaborator

🤔 So the last two changes fix our CI for docs and the Read the docs build but, of course, break when we try and install the "dev" Sphinx version from GitHub https://github.com/pydata/pydata-sphinx-theme/actions/runs/8741568491/job/23987894819?pr=1772#step:5:76

@drammock
Copy link
Collaborator Author

🤔 So the last two changes fix our CI for docs and the Read the docs build but, of course, break when we try and install the "dev" Sphinx version from GitHub https://github.com/pydata/pydata-sphinx-theme/actions/runs/8741568491/job/23987894819?pr=1772#step:5:76

One way to get the CI to pass is to revert the pin in pyproject.toml, and add an extra step in the doc build workflow to downgrade sphinx after the initial package install is done. But: we do kinda need the pin in pyproject.toml to avoid breaking user sites that use ablog so I guess we either:

  1. let the "sphinx-dev" job fail for a while
  2. change it to not actually test sphinx dev, and hopefully remember to change it back later

@nabobalis
Copy link

nabobalis commented Apr 18, 2024

Sorry about ablog, I am in the process of patching it and doing a release when/if my CI passes.

Edit: I have tagged the release, it should be out on pypi shortly

@trallard
Copy link
Collaborator

Ok I can try with the upcoming tag in Ablog.

Actually testing against dev helps identify breaking changes against Sphinx. So it served it's purpose even if it gave us a bit of a headache.

As I am working on the CI improvements I see a path here:

  1. Pin the Sphinx version we use in pyproject.toml - this way we ensure we do not break things for users (might need to do this for other dependencies too to avoid compatibility issues moving forward)
  2. Separately in CI - I can add a workflow that runs every so often to try and bump our dependencies and run tests and build against those bumped versions - if this works then we can safely upgrade our dependencies avoiding breaking changes for our users

WDYT @drammock

@drammock
Copy link
Collaborator Author

Yeah a periodic bump test sounds good. Thanks @trallard

@drammock
Copy link
Collaborator Author

But so we then keep in the failing dev test? Or hope/ wait until it works with new ablog version

@trallard
Copy link
Collaborator

trallard commented Apr 18, 2024

I am logging off but I can try with the newest Ablog release tomorrow or try a quick fix to uninstall/install the Dev sphinx version while the bigger CI overhaul gets finished

@Carreau
Copy link
Collaborator

Carreau commented Apr 19, 2024

There is both a new version of sphinx that will temporarily fix the crash (though it might affect ablog's configuration), and a blog that will work if sphinx ever raise an error again.

@drammock
Copy link
Collaborator Author

OK everything is passing except CodeCov upload now! @trallard @Carreau ready for review/merge.

Copy link
Collaborator

@trallard trallard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, will go ahead and merge and work on the other CI PR so we can merge that one quickly too

@trallard trallard merged commit ee322a7 into pydata:main Apr 19, 2024
16 of 17 checks passed
@drammock drammock deleted the defusedxml-again branch April 23, 2024 21:35
gabalafou pushed a commit to gabalafou/pydata-sphinx-theme that referenced this pull request Apr 29, 2024
* CI: install sphinx test deps always

* fix it right this time

* pin sphinx<7.3 for site builds

* Fix Sphinx version

* Pin Sphinx in pyproject.toml

* bump ablog; unpin sphinx

---------

Co-authored-by: Tania Allard <taniar.allard@gmail.com>
ivanov pushed a commit to ivanov/pydata-sphinx-theme that referenced this pull request Jun 5, 2024
* CI: install sphinx test deps always

* fix it right this time

* pin sphinx<7.3 for site builds

* Fix Sphinx version

* Pin Sphinx in pyproject.toml

* bump ablog; unpin sphinx

---------

Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants