-
Notifications
You must be signed in to change notification settings - Fork 94
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
Add Python 3.9 for unit tests in CI (experimental, do not stop other builds if it fails) #3854
Conversation
4cc816d
to
08df9be
Compare
(Should fail for now as I think GH hasn't added support to 3.9 yet) |
The bash container is failing to build due to… virtual memory I think?
|
Ha! https://docs.docker.com/engine/reference/run/#runtime-constraints-on-resources Interesting, I never had to limit memory utilization of containers. Today-I-Learned 🎉 |
docker build -t bash:test dockerfiles/bash/ | ||
docker run --name bash -v $(pwd -P):/root/cylc-flow --rm -t -d bash:test /bin/bash | ||
docker build -t bash:test -m 1G --memory-swap 1G dockerfiles/bash/ | ||
docker run --name bash -v $(pwd -P):/root/cylc-flow --rm -t -d -m 1G --memory-swap 1G bash:test /bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was getting some errors due to low virtual memory when building the container. Not sure if others have seen this, but might be good to constraint resources so our builds don't fail sporadically.
(low priority, happy to leave this one waiting until GH has 3.9 support if others prefer 👍 ) |
experimental: [false] | ||
include: | ||
- python-version: 3.9 | ||
experimental: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are python-version
and experimental
lists above, but scalar values here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hadn't noticed it. Both appear to work fine. But let me edit that commit to match the other values 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, just need to wait for CI to run and confirm it works too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I see the GH Actions docs does it the way you originally did it, so maybe it works either way 🤞 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bugger:
Error: .github/workflows/test.yml (Line: 25, Col: 27): A sequence was not expected
Error: The template is not valid. .github/workflows/test.yml (Line: 25, Col: 27): A sequence was not expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But good to know! I guess if you have multiple versions, then you will have to do something like
- python-version: 1
experimental: true
- python-version: 2
experimental: true
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the "experimental" mean exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a feature in GitHub actions that allows to mark a branch of the matrix-tree as not-ready I guess? It's really common in Java and JavaScript, for new versions of the JVM or of Node (which are experimental in relation to the code; meaning that they may not work, and the developers/CI are "experimenting" with it? 😄 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fair enough. Just metadata then, no real consequences 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove the experimental thing now 3.9 is out.
10048f4
to
b8a4821
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3.9 block fails, as expected. Any opinions on whether or not we should merge like this or wait till GH-A adds 3.9 support? (It might look a bit odd to have expected test fails all the time?)
We can review/approve and merge later. They have some pull requests in GH actions virtual environments repo for Ubuntu 18.04 and 20.04, but the next updates don't seem to include Python 3.9. I think first it needs to make its way to Ubuntu package repositories, then they will create a PR to merge? https://github.com/actions/virtual-environments/pulls |
I'll label this as "blocked" for now, pending 3.9 on GH-A. |
4a62ed6
to
b8a4821
Compare
Ignore last commits. I added one commit to test something I saw in GH actions docs for setup-python:
Didn't work 😬 reverted |
b8a4821
to
496c211
Compare
I think the 3.9 unit tests will pass now, actions/setup-python#148 |
CI passed, two approvals. Merging 🎉 |
This is a small change with no associated Issue.
This is how GH actions handles experimental Java versions. I guess it should work with Python versions too…
Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.