Skip to content

[WIP][Java] Add libcuvs version check #1315

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[WIP][Java] Add libcuvs version check #1315

wants to merge 2 commits into from

Conversation

@ldematte
Copy link
Contributor

@ldematte ldematte commented Sep 5, 2025

This PR extends #1314 to add version check (version reported by libcuvs vs. cuvs-java build version)

@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 5, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

ChrisHegarty
ChrisHegarty reviewed Sep 5, 2025
View reviewed changes
@cjnolet cjnolet added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Sep 5, 2025
@mythrocks
Copy link
Contributor

mythrocks commented Sep 5, 2025

For the record, I took this change (and #1314) out for a spin with #1296. The patches don't apply completely cleanly, but they are easy enough to resolve.

Things seem to work. These changes are compatible with #1296. I also tested changing the version, to see if the incompatibility is detected. That works as well.

mythrocks
mythrocks approved these changes Sep 8, 2025
View reviewed changes
Copy link
Contributor

@mythrocks mythrocks left a comment

Choose a reason for hiding this comment

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

I'm approving this change, and #1314. I'll schedule these to go after #1296, after rebase.

@ldematte
Copy link
Contributor Author

ldematte commented Sep 9, 2025

I'm approving this change, and #1314. I'll schedule these to go after #1296, after rebase.

++, I will handle the rebase.

@mythrocks mythrocks mentioned this pull request Sep 10, 2025
Merged
@ldematte ldematte changed the base branch from branch-25.10 to pull-request/1314 September 11, 2025 06:25
@copy-pr-bot copy-pr-bot bot deleted the branch rapidsai:pull-request/1314 September 11, 2025 14:46
@copy-pr-bot copy-pr-bot bot closed this Sep 11, 2025
@ldematte
Copy link
Contributor Author

ldematte commented Sep 11, 2025

@mythrocks I don't understand why this was closed.. can you help?

@mythrocks
Copy link
Contributor

mythrocks commented Sep 11, 2025

Hmm. I don't get this either. It's not allowing me to reopen it either. Very odd.

@mythrocks
Copy link
Contributor

mythrocks commented Sep 11, 2025 •
edited
Loading

Reopening, again.

Nope, no dice. The Github UI says that the branch pull-request/1314 has been deleted. The problem seems to have happened after merging #1314.

Please feel free to raise another PR with the same branch. It's already been reviewed, so it shouldn't be controversial to get it in.

@mythrocks
Copy link
Contributor

mythrocks commented Sep 11, 2025

Ah, I see that this PR was raised against pull-request/1314, which went away after the merge. That's what happened.

@ldematte
Copy link
Contributor Author

ldematte commented Sep 12, 2025

Ah, I see that this PR was raised against pull-request/1314, which went away after the merge. That's what happened.

yep, I wanted to change it to branch-25.10 but it does not let you do that when the PR is closed, and if it is closed again as soon as you re-open it...
I'll raise a new PR.

@ldematte ldematte mentioned this pull request Sep 12, 2025
Merged
@ldematte
Copy link
Contributor Author

ldematte commented Sep 12, 2025

I've create #1327 to replace this

@ldematte ldematte mentioned this pull request Sep 12, 2025
Merged
rapids-bot bot pushed a commit that referenced this pull request Sep 14, 2025
@ldematte
[Java] Add libcuvs <-> cuvs-java version check (#1327)
e292a09 
This PR extends #1314 to add version check (version reported by libcuvs vs. cuvs-java build version)

It replaces #1315 which was already reviewed and approved, but got closed before I could rebase it.

Authors:
  - Lorenzo Dematté (https://github.com/ldematte)

Approvers:
  - MithunR (https://github.com/mythrocks)

URL: #1327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythrocks mythrocks mythrocks approved these changes

+1 more reviewer

@ChrisHegarty ChrisHegarty ChrisHegarty left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ldematte ldematte

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

Vector Search, ML, & Data Mining Release Board
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ldematte @mythrocks @ChrisHegarty @cjnolet