-
Notifications
You must be signed in to change notification settings - Fork 10
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
CI: Add test for uploads with multiple labels #54
Conversation
* Add test uploads for non-main label 'test' and for multiple labels in a comma seperated list. As attempting to apply multiple labels through repeated upload of the same package will fail as the package already exists, bump the version number of the test package to generate multiple packages that can be uploaded with different labels.
ce721d8
to
f41df7e
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.
If we wanted to also check what labels are uploaded as a validation step in cleanup
before removing them we could add something like
curl --silent https://api.anaconda.org/package/scientific-python-nightly-wheels/test-package | \
jq -r '.releases[].version' > package-versions.txt
for package_version in $(cat package-versions.txt); do
echo "# scientific-python-nightly-wheels/test-package version ${package_version} labels:"
curl --silent https://api.anaconda.org/package/scientific-python-nightly-wheels/test-package | \
jq -r '.files[] | \
select( .version == "${package_version}" )' | \
jq -r '.labels'
done
but I don't think that's strictly necessary, so I've left this out.
run: | | ||
# Bump version to avoid wheel name conflicts | ||
sed -i 's/0.0.1/0.0.2/g' tests/test_package/pyproject.toml | ||
rm ./dist/* | ||
python -m build --outdir ./dist tests/test_package |
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.
As attempting to apply multiple labels through repeated upload of the same package will fail as the package already exists, bump the version number of the test package to generate multiple packages that can be uploaded with different labels.
@@ -36,7 +36,7 @@ jobs: | |||
|
|||
- name: Build a wheel and a sdist | |||
run: | | |||
PYTHONWARNINGS=error,default::DeprecationWarning python -m build --outdir ./dist tests/test_package | |||
python -m build --outdir ./dist tests/test_package |
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.
Removing this as I had added in previously from another workflow I have, but we don't care if there are deprecation warnings in the package build tools, as we just need an artifact to upload.
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.
that assumes that the command will keep working as is, which I think is a valid assumption (or will be a super simple fix many years down the line), so 👍
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.
Yeah, the main reason I removed this is that I've had this check in other work flows and this has ended up causing things to fail for reasons unrelated to build
itself (e.g. a build backend deprecating something for the future but that warning still getting caught). I agree that if build
ends up having issues though we will not be the first to catch 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.
I only have extreme nitpicks, and also wondered about the same thing you wrote whether the labels need to be verified. I don't necessarily think, so would say go ahead and merge this without that check.
@@ -36,7 +36,7 @@ jobs: | |||
|
|||
- name: Build a wheel and a sdist | |||
run: | | |||
PYTHONWARNINGS=error,default::DeprecationWarning python -m build --outdir ./dist tests/test_package | |||
python -m build --outdir ./dist tests/test_package |
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.
that assumes that the command will keep working as is, which I think is a valid assumption (or will be a super simple fix many years down the line), so 👍
Co-authored-by: Brigitta Sipőcz <b.sipocz@gmail.com>
Co-authored-by: Brigitta Sipőcz <b.sipocz@gmail.com>
Add test uploads for non-main label 'test' and for multiple labels in a comma seperated list to compliment the feature added in PR #47.
This is an example of things working as intended prior to the cleanup step removing the test package.