-
-
Notifications
You must be signed in to change notification settings - Fork 551
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
Failing release-docs
workflow for release 0.22.0
#1649
Comments
Hey @gbolmier! Thanks for taking a look. Honestly I would say it's ok to drop that last part of the notebook that uses SpaCy. It's not a big deal. We can always revisit and add it back later. |
In the end, didn't have to drop the spaCy part, incrementing poetry cache key in install-env action fixed it. The release should be out in 3-4 hours (time for the PyPI job to finish) |
Great job @gbolmier, thank you very much 🙏 |
Thanks guys! Got some further issues with the pypi workflow but hopefully the current run is the right one 🤞 |
I'd recommend splitting off the QEMU-based builds into a separate job or add a matrix, that might be faster for future iterations ;) Also, setting |
Another thing: I notice that the wheel builds are running for multiple images under the same operating system: while I understand the inclusion of the Windows 2019/2022 and macOS 13/14 GHA images as separate builds because of different compiler toolchains (MSVC/Xcode) installed across the images (which might be slightly redundant too, though) – the trio of Ubuntu 20.04/22.04/24.04 is most likely redundant because |
Great suggestions @agriyakhetarpal, thanks for sharing! I'm actually not used to work on this kind of stuff so any feedback is quite valuable to me.
Our workflow implementation uses a matrix to parallelize the builds by os. QEMU is used for the ubuntu builds. The current bottleneck is coming from the
+1 for switching to
Oh, good catch! If the wheels are the same then yeah, let's only keep Hopefully the current workflow run finishes successfully in an hour and I can open a PR for the next time. Does that sound good @MaxHalford? |
I'm glad you like it! I'm quite used to doing things like this, so happy to look around and offer more!
Sounds good to me, and yes, removing the explicit pin and using
How about we parallelise at a higher granularity, i.e., both at the Python version level and at the glibc/musl level (for Linux)? We can do this with a
Happy to review that PR or more if you'd like me to! P.S. cibuildwheel now sets the minimum macOS deployment target as 10.13 by default (bumped from the previous 10.9) since a few releases now, so it is not needed to set it explicitly during wheel repair. :D |
Yep sounds good. Thanks for shedding some light on this topic @agriyakhetarpal, it's not an obvious topic! |
The release is finally out 🥳 Thanks a lot for your help @agriyakhetarpal! I've actually already implemented and pushed most of the changes we were talking about (the 12 jobs split, and only I removed the |
Thank you! 🎉 Good to see the split – indeed, 56 minutes in total for all the jobs included! Yes, Would y'all be interested in adding an out-of-tree Emscripten/Pyodide CI job for |
Yes, @MaxHalford took the initiative a year ago (see #1290). The last entry of our making a release instructions is:
But it looks like only River If I understand your suggestion correctly, would adding something similar to the following job to our existing workflow be enough and better? (not sure for the python versions, may require a matrix) Snippet build_pyodide_wheels:
name: Build Pyodide wheels
needs: [build_macos_wheels]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up rust
uses: dtolnay/rust-toolchain@stable
with:
toolchain: nightly
- name: Build wheels
uses: pypa/cibuildwheel@v2.22.0
timeout-minutes: 720
env:
CIBW_BUILD: "cp310-* cp311-* cp312-* cp313-*"
CIBW_PLATFORM: pyodide
CIBW_ENVIRONMENT: 'PATH="$HOME/.cargo/bin:$PATH"'
CIBW_ENVIRONMENT_LINUX: 'PATH="$HOME/.cargo/bin:$PATH" CARGO_NET_GIT_FETCH_WITH_CLI="true"'
CIBW_BEFORE_BUILD: >
rustup default nightly &&
rustup show
CIBW_BEFORE_BUILD_LINUX: >
curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain=nightly --profile=minimal -y &&
rustup show
- uses: actions/upload-artifact@v4
with:
name: artifact-pyodide
path: ./wheelhouse/*.whl |
That job should work perfectly for building WASM wheels, @gbolmier, but it won't test them – you should also add the For greater flexibility, I'd also recommend looking at the previous iteration of this CI job for NumPy – which would be numpy/numpy#25894. In-tree, we usually (try to) upgrade all packages periodically when we update the Emscripten version (which is when we have an ABI break). There is a chance |
Thanks for all the info! I'll try to find the time to open a PR tomorrow evening or in the next days. |
I've been trying to make the new
0.22.0
release today but I've been facing some issues. All the workflows are now running properly except therelease-docs
one.The docs environment installs spaCy for the sentence-classification nb example, unfortunately spaCy does not support python 3.13 yet (see explosion/spaCy#13658), apparently because of its dependency to srsly (see explosion/srsly#112).
I tried temporarily downgrading the python version in the docs workflows (
dev-docs
andrelease-docs
) but therelease-docs
one still fails (see run).I may not have the time to investigate more tomorrow so if someone want to give it a try by then, please do.
Cc @AdilZouitine, @MaxHalford, @smastelini, @agriyakhetarpal
The text was updated successfully, but these errors were encountered: