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

update RAPIDS dependencies to 24.4, refactor dependencies.yaml #5726

Merged
merged 8 commits into from
Jan 24, 2024

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Jan 22, 2024

Proposes updating all remaining 24.2.* dependencies to 24.4.*.

I believe that's the desired behavior for the branch-24.04 branch and that these were just missed in 6e76da4#diff-5475a6d76de4c506ee92cf6f941bba30a6a07b5881cfea531d42a6ec035095a6.

This also pulls in some dependency refactoring originally added in #5711, which allows greater use of dependencies.yaml globs (and therefore less maintenance effort to support new CUDA versions).

@jameslamb jameslamb requested review from a team as code owners January 22, 2024 19:06
@github-actions github-actions bot added conda conda issue Cython / Python Cython or Python issue labels Jan 22, 2024
@bdice
Copy link
Contributor

bdice commented Jan 22, 2024

@jameslamb You uncovered a real issue, I just found the same thing with rapidsai/ucxx#171 and it affects a bunch of repos across RAPIDS.

Context: We've been migrating our dependencies.yaml to specify RAPIDS packages in separate depends_on_X lists like this with separate matrix entries for the -cu11 and -cu12 packages. This separation of lists for RAPIDS packages helps with making devcontainers / conda / pip all happy, and centralizes some of the things like extra index URLs that need to be added. However, we haven't aligned ci/release/update-version.sh to accommodate those suffixed entries. We probably need to find a strategy for unifying this across RAPIDS. I would suggest that we experiment with this sed command to ensure it covers the suffixed entries. This is probably a good candidate for an issue in https://github.com/rapidsai/build-planning so we can determine a strategy and sweep through all of RAPIDS.

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

This PR needs some updates to ci/release/update-version.sh to prevent this from reoccurring. We might also want to port over some of the dependency refactoring in #5711 (some issues in dependencies.yaml were fixed there) so we only have to fix update-version.sh once, but I'll leave that choice to you.

@github-actions github-actions bot added the ci label Jan 22, 2024
@raydouglass
Copy link
Member

@jameslamb You uncovered a real issue, I just found the same thing with rapidsai/ucxx#171 and it affects a bunch of repos across RAPIDS.

Looks like just cudf & cuml were affected during creating branch-24.04. Un-merged PRs could be affected too.

I opened rapidsai/cudf#14825 to fix it in cudf. But something more robust across all of RAPIDS is a good idea.

@raydouglass raydouglass added bug Something isn't working non-breaking Non-breaking change labels Jan 22, 2024
@jameslamb jameslamb changed the title update RAPIDS dependencies to 24.4 update RAPIDS dependencies to 24.4, refactor dependencies.yaml Jan 22, 2024
@github-actions github-actions bot removed conda conda issue Cython / Python Cython or Python issue labels Jan 22, 2024
@jameslamb
Copy link
Member Author

This is probably a good candidate for an issue in https://github.com/rapidsai/build-planning so we can determine a strategy and sweep through all of RAPIDS.
...
... something more robust across all of RAPIDS is a good idea.

I'll open an issue in build-planning @bdice @raydouglass .

This PR needs some updates to ci/release/update-version.sh to prevent this from reoccurring. We might also want to port over some of the dependency refactoring in #5711

I just pushed commits that do both these things, and re-requested a review.

@jameslamb jameslamb requested a review from bdice January 22, 2024 19:45
ci/release/update-version.sh Outdated Show resolved Hide resolved
@github-actions github-actions bot added conda conda issue Cython / Python Cython or Python issue labels Jan 22, 2024
@jameslamb
Copy link
Member Author

This devcontainer job failure looks like a permissions issue to me.

 -- CPM: Adding package [email protected] (branch-24.04)
  
  [1/9] Creating directories for 'cumlprims_mg-populate'
  [1/9] Performing download step (git clone) for 'cumlprims_mg-populate'
  Cloning into 'cumlprims_mg-src'...
  Host key verification failed.
  fatal: Could not read from remote repository.
  
  Please make sure you have the correct access rights
  and the repository exists.
  Cloning into 'cumlprims_mg-src'...
  Host key verification failed.
  fatal: Could not read from remote repository.
  
  Please make sure you have the correct access rights
  and the repository exists.
  Cloning into 'cumlprims_mg-src'...
  Host key verification failed.
  fatal: Could not read from remote repository.
  
  Please make sure you have the correct access rights
  and the repository exists.
  -- Had to git clone more than once: 3 times.
  CMake Error at cumlprims_mg-subbuild/cumlprims_mg-populate-prefix/tmp/cumlprims_mg-populate-gitclone.cmake:39 (message):
    Failed to clone repository: '[email protected]:rapidsai/cumlprims_mg.git'

(build link)

@jameslamb
Copy link
Member Author

Maybe this deploy key has been removed?

extra-repo-deploy-key: CUMLPRIMS_SSH_PRIVATE_DEPLOY_KEY

@raydouglass is that something you could check?

@jameslamb
Copy link
Member Author

Discussed offline w/ @raydouglass

I misinterpreted that erorr "Host key verification failed." It's not about this repo's SSH keys... it's about the key fingerprint of the github.com host.

We suspect this was a temporary disruption on GitHub's side and that a re-run of CI here will resolve the failing devcontainer build (build link).

Comment on lines +159 to +160
- &pylibraft_conda pylibraft==24.4.*
- &rmm_conda rmm==24.4.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know why were these missed by update-version.sh previously? They're not -cu11 or -cu12 names. Was it due to a forward-merge after branch-24.04 was created?

Copy link
Member

Choose a reason for hiding this comment

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

The &rmm_conda part prevented being updated. That's why the line in update-version.sh added the .* as in -.* ${DEP}...

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense to me!

The presence of - in these commands also explains why some projects have separate lines for updating pyproject.toml files, like this:

https://github.com/rapidsai/cugraph/blob/f20219ed62aaade9ff21e5867ea828ee20baef9b/ci/release/update-version.sh#L97-L107

@jameslamb
Copy link
Member Author

Looks like the devcontainers build is now passing thanks to the changes in rapidsai/devcontainers#214.

Thanks @trxcllnt !!!

@AyodeAwe
Copy link
Contributor

/merge

@AyodeAwe AyodeAwe merged commit 6186e84 into rapidsai:branch-24.04 Jan 24, 2024
55 checks passed
@AyodeAwe
Copy link
Contributor

Admin merged on author's request

@jameslamb jameslamb deleted the more-rapids-24.4-updates branch January 24, 2024 15:04
@jakirkham
Copy link
Member

Thanks all! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ci conda conda issue Cython / Python Cython or Python issue non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants