-
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
ENH: Add optional anaconda_nightly_upload_organization
argument
#47
Changes from all commits
e50044e
66d2324
317d7cb
9740248
9b762da
97a6a9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,8 +14,17 @@ set -x | |
# this is to prevent accidental uploads | ||
echo "Getting anaconda token from github secrets..." | ||
|
||
ANACONDA_ORG="scientific-python-nightly-wheels" | ||
ANACONDA_ORG="${INPUT_ANACONDA_NIGHTLY_UPLOAD_ORGANIZATION}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would this break the current behaviour for the core libraries uploading to the default location? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, because I added the default location in this line: https://github.com/scientific-python/upload-nightly-action/pull/47/files#diff-1243c5424efaaa19bd8e813c5e6f6da46316e63761421b3e5f5c8ced9a36e6b6R20 However, I believe that:
Thank you for your review @bsipocz! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @skoudoro While I'm not against the idea of being explicit about the target channel (I think that's a good idea in general to guard against default changes), I think the original intent for the GitHub Action was to make uploading as easy as possible for the core packages with the idea that they need to configure things as minimally as possible. I think that this could probably be discussed more broadly outside of this PR though, so maybe you can open up an Issue from your comment to discuss what the target audience and scope of the action configuration should be? |
||
ANACONDA_TOKEN="${INPUT_ANACONDA_NIGHTLY_UPLOAD_TOKEN}" | ||
ANACONDA_LABELS="${INPUT_ANACONDA_NIGHTLY_UPLOAD_LABELS}" | ||
|
||
# if the ANACONDA_ORG is empty, exit with status -1 | ||
# this is to prevent attempt to upload to the wrong anaconda channel | ||
if [ -z "${ANACONDA_ORG}" ]; then | ||
echo "ANACONDA_ORG is empty, exiting..." | ||
exit -1 | ||
fi | ||
|
||
|
||
# if the ANACONDA_TOKEN is empty, exit with status -1 | ||
# this is to prevent accidental uploads | ||
|
@@ -24,6 +33,25 @@ if [ -z "${ANACONDA_TOKEN}" ]; then | |
exit -1 | ||
fi | ||
|
||
# if the ANACONDA_LABELS is empty, exit with status -1 | ||
# as this should be set in action.yml or by the user | ||
# and it is better to fail on this to sigal a problem. | ||
if [ -z "${ANACONDA_LABELS}" ]; then | ||
echo "ANACONDA_LABELS is empty, exiting..." | ||
exit -1 | ||
fi | ||
|
||
# convert newlines to commas for parsing | ||
# and ensure that there is no trailing comma | ||
ANACONDA_LABELS="$(tr '\n' ',' <<< "${ANACONDA_LABELS}" | sed 's/,$//')" | ||
|
||
IFS=',' read -ra LABELS <<< "${ANACONDA_LABELS}" | ||
|
||
LABEL_ARGS="" | ||
for label in "${LABELS[@]}"; do | ||
LABEL_ARGS+="--label ${label} " | ||
done | ||
|
||
# Install anaconda-client from lock file | ||
echo "Installing anaconda-client from upload-nightly-action conda-lock lock file..." | ||
micromamba create \ | ||
|
@@ -48,5 +76,6 @@ echo "Uploading wheels to anaconda.org..." | |
anaconda --token "${ANACONDA_TOKEN}" upload \ | ||
--force \ | ||
--user "${ANACONDA_ORG}" \ | ||
$ANACONDA_LABELS \ | ||
"${INPUT_ARTIFACTS_PATH}"/*.whl | ||
echo "Index: https://pypi.anaconda.org/${ANACONDA_ORG}/simple" |
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 don't know how the action works, but this is confusing since it isn't a URL.
If it can be anything other than a URL, that should either be documented or the field be renamed.
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.
So the variable name is wrong! it should be
anaconda_nightly_upload_organization
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 the default will break downstream packages using this action as soon as it is merged like scikit-image
scikit-image is using main. Maybe we can update this in a new PR and let some time to all downstream package to add/update this variable.