Query UCX-Py from gpuCI versioning service #818
Conversation
pentschev
added
3 - Ready for Review
There was a problem hiding this comment.
As discussed in our Slack thread, we should move the curl call to the script below (which gets run every time a new release branch is created) in order to minimize the number of network requests in CI. Similarly to cudf (link), we can then use some sed commands to find and replace any hardcoded values that need to be updated in the repo.
https://github.com/rapidsai/dask-cuda/blob/branch-22.02/ci/release/update-version.sh
|
@ajschmidt8 I updated scripts to reflect the change you suggested, could you take a look to ensure I did everything correctly? |
ci/release/update-version.sh
Outdated
| @@ -17,11 +17,13 @@ CURRENT_MAJOR=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[1]}') | |||
| CURRENT_MINOR=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[2]}') | |||
| CURRENT_PATCH=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[3]}') | |||
| CURRENT_SHORT_TAG=${CURRENT_MAJOR}.${CURRENT_MINOR} | |||
| CURRENT_UCXPY_VERSION=$(curl -s https://version.gpuci.io/rapids/${CURRENT_SHORT_TAG}`.*) | |||
There was a problem hiding this comment.
Not currently, I had it for completeness but I removed now since it's unused at this time.
ci/release/update-version.sh
Outdated
|
|
||
| #Get <major>.<minor> for next version | ||
| NEXT_MAJOR=$(echo $NEXT_FULL_TAG | awk '{split($0, a, "."); print a[1]}') | ||
| NEXT_MINOR=$(echo $NEXT_FULL_TAG | awk '{split($0, a, "."); print a[2]}') | ||
| NEXT_SHORT_TAG=${NEXT_MAJOR}.${NEXT_MINOR} | ||
| NEXT_UCXPY_VERSION=$(curl -s https://version.gpuci.io/rapids/${NEXT_SHORT_TAG}`.*) |
There was a problem hiding this comment.
I think this should be NEXT_UCXPY_VERSION="$(curl -s https://version.gpuci.io/rapids/${NEXT_SHORT_TAG}).*"
There was a problem hiding this comment.
Agreed, I now see there were two typos, your suggestion should fix both. Thanks @Ethyling !
ci/release/update-version.sh
Outdated
| # Update UCX-Py version | ||
| sed_runner 's/export UCXPY_VERSION=.*/export UCXPY_VERSION='${NEXT_UCXPY_VERSION}'/g' ci/gpu/build.sh |
There was a problem hiding this comment.
I think we should use " instead of ' to expand the variable
There was a problem hiding this comment.
No objections from me, I had copied the format from cuDF's script, but I now see it mixes both formats. For example https://github.com/rapidsai/cudf/blob/67c925c84aa111ad3a54c0352c18c570d17f5d75/ci/release/update-version.sh#L34 and https://github.com/rapidsai/cudf/blob/67c925c84aa111ad3a54c0352c18c570d17f5d75/ci/release/update-version.sh#L58
Sorry, late to reply here. Looks like Jordan's got you covered! |
|
Sorry for the late reply here as well, I think all suggestions are now addressed. Would you mind taking another look when you have the chance @Ethyling @ajschmidt8 ? |
|
rerun tests |
1 similar comment
|
rerun tests |
|
Forgot to merge this, thanks @ajschmidt8 and @Ethyling for reviews! |
|
@gpucibot merge |
No description provided.