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

feat: Search for & upload .conda packages #37

Closed
wants to merge 4 commits into from

Conversation

jakirkham
Copy link
Member

Part of issue: rapidsai/build-planning#98

  • Uses find's -o option to search for .conda or .tar.bz2 files to upload
  • Tack on .conda glob to anaconda upload command

@jakirkham jakirkham changed the title Upload dot conda feat: Search for & upload .conda packages Aug 29, 2024
Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I left one suggestion for your consideration.

ci/gpuci/run.sh Outdated
Comment on lines 25 to 28
anaconda -t ${MY_UPLOAD_KEY} upload -u ${CONDA_USERNAME:-gpuci} --label main --skip-existing \
/opt/conda/conda-bld/${ARCH_DIR}/gpuci-tools*.conda \
/opt/conda/conda-bld/${ARCH_DIR}/gpuci-tools*.tar.bz2 \
;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
anaconda -t ${MY_UPLOAD_KEY} upload -u ${CONDA_USERNAME:-gpuci} --label main --skip-existing \
/opt/conda/conda-bld/${ARCH_DIR}/gpuci-tools*.conda \
/opt/conda/conda-bld/${ARCH_DIR}/gpuci-tools*.tar.bz2 \
;
anaconda -t ${MY_UPLOAD_KEY} upload -u ${CONDA_USERNAME:-gpuci} --label main --skip-existing /opt/conda/conda-bld/${ARCH_DIR}/gpuci-tools*.{conda, tar.bz2}

Think it'd be a bit simpler to just use {option1,option2} instead of repeating the search directory twice.

I'd still support breaking that onto another line if you want, but don't feel as strongly about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly was thinking of replacing this by find as currently shellcheck doesn't like the lack of quotes

Copy link
Member

Choose a reason for hiding this comment

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

that's fine too, whatever works 🤷🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it was just CI was failing due to shellcheck. So that also needed to be addressed

Though do appreciate the tip

Please let me know what you think of the rewrite

* Match other upload scripts with `find`
* Do a single pass of the files
* Write in a `shellcheck` conformant way
@jakirkham
Copy link
Member Author

jakirkham commented Aug 29, 2024

Looks like remain errors are outside of the file changed here. From CI:

./tools/gpuci_retry:42:13: note: Double quote to prevent globbing and word splitting. [SC2086]

Edit - Handling this in PR: #38

Copy link
Member

@jameslamb jameslamb 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, thank you!

@jakirkham
Copy link
Member Author

After talking with Ray offline, it sounds like this repo is scheduled for archival. So we don't need this PR

@jakirkham
Copy link
Member Author

To ensure there are no more references to the script changed in this PR, searched the org specifically for rapids-upload-to-anaconda, there are 3 references:

So there are no other references to rapids-upload-to-anaconda specifically

Meaning we don’t need this PR. So am closing it out

@jakirkham jakirkham closed this Sep 26, 2024
@jakirkham jakirkham deleted the upload_dot_conda branch September 26, 2024 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants