Skip to content

align service api with SDK (compat with deprecations) #227

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
Merged
rapids-bot merged 22 commits into from
Jul 29, 2025
Merged

align service api with SDK (compat with deprecations) #227

rapids-bot merged 22 commits into from
Jul 29, 2025

Conversation

@tmckayus
Copy link
Contributor

@tmckayus tmckayus commented Jul 23, 2025

The service API is not aligned completely with the SDK API from 25.05 in relation to solver configs. This change brings them into alignment with deprecation notices and backward compatibility

@tmckayus tmckayus requested a review from a team as a code owner July 23, 2025 14:17
@tmckayus tmckayus requested a review from Iroy30 July 23, 2025 14:17
@tmckayus tmckayus added bug Something isn't working non-breaking Introduces a non-breaking change labels Jul 23, 2025
@tmckayus tmckayus changed the title [WIP] align service api with SDK (compat with deprecations) align service api with SDK (compat with deprecations) Jul 23, 2025
@tmckayus
Copy link
Contributor Author

tmckayus commented Jul 23, 2025

closes #193
closes #197

@tmckayus
Copy link
Contributor Author

tmckayus commented Jul 24, 2025

realized that on the clientside, ThinclientSolverSettings also needs to be fixed up

rgsl888prabhu
rgsl888prabhu reviewed Jul 24, 2025
View reviewed changes
Copy link
Collaborator

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

CUOPT_MIP_HEURISTICS_ONLY needs to be updated in docs section.

Also lets add tests to make sure the naming changes are in good shape.

@tmckayus tmckayus requested a review from a team as a code owner July 25, 2025 18:09
@tmckayus tmckayus requested a review from gforsyth July 25, 2025 18:09
@tmckayus
Copy link
Contributor Author

tmckayus commented Jul 25, 2025

CUOPT_MIP_HEURISTICS_ONLY needs to be updated in docs section.

Also lets add tests to make sure the naming changes are in good shape.

ok fixed the doc and added a dataset that sets all solver_config values for LP to make sure none are rejected
Testing the precedence of current vs deprecated I think is overkill.

rgsl888prabhu
rgsl888prabhu reviewed Jul 28, 2025
View reviewed changes
Copy link
Collaborator

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

Awesome @tmckayus, changes look good.

Have few minor doc related suggestions, I am approving the PR. So you can push those changes and merge once it is ready.

@rgsl888prabhu rgsl888prabhu added this to the 25.08 milestone Jul 28, 2025
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 29, 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.

@tmckayus
Copy link
Contributor Author

tmckayus commented Jul 29, 2025

/ok to test 03966cf

@tmckayus
Copy link
Contributor Author

tmckayus commented Jul 29, 2025

/merge

@rapids-bot rapids-bot bot merged commit 8dc24a7 into NVIDIA:branch-25.08 Jul 29, 2025
210 of 215 checks passed
@rgsl888prabhu rgsl888prabhu mentioned this pull request Aug 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gforsyth gforsyth gforsyth approved these changes

@rgsl888prabhu rgsl888prabhu rgsl888prabhu approved these changes

@Iroy30 Iroy30 Awaiting requested review from Iroy30 Iroy30 is a code owner automatically assigned from NVIDIA/cuopt-infra-codeowners

@chris-maes chris-maes Awaiting requested review from chris-maes

Assignees

No one assigned

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Milestone

25.08

Development

Successfully merging this pull request may close these issues.

3 participants

@tmckayus @gforsyth @rgsl888prabhu