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

Remove client testing of server trace to match discontinued support from server #7129

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

matthewkotila
Copy link
Contributor

Trace settings update from client support was discontinued in #7106

This PR removes testing in PA of trace settings updating from client.

@matthewkotila matthewkotila requested a review from ganeshku1 April 17, 2024 21:02
Copy link
Contributor

@dyastremsky dyastremsky left a comment

Choose a reason for hiding this comment

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

Thanks for this fix!

Copy link
Contributor

@ganeshku1 ganeshku1 left a comment

Choose a reason for hiding this comment

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

Sorry saw this PR after it was approved.
Do we need any doc changes for this trace option removal?
Or is that being handled as part of a separate PR?

@matthewkotila
Copy link
Contributor Author

@ganeshku1: Sorry saw this PR after it was approved.
Do we need any doc changes for this trace option removal?
Or is that being handled as part of a separate PR?

Separate PR. I think @dyastremsky can speak more to that: triton-inference-server/client@main...dyas-trace

@matthewkotila matthewkotila merged commit 1dc0430 into main Apr 17, 2024
3 checks passed
@matthewkotila matthewkotila deleted the matthewkotila-fix-pa-trace branch April 17, 2024 22:07
@dyastremsky
Copy link
Contributor

Sorry saw this PR after it was approved. Do we need any doc changes for this trace option removal? Or is that being handled as part of a separate PR?

Thanks for tagging me, Matt. Yes, I'm making these changes. The PR got a bit stalled due to a failing CI. Hoping this next run resolves the issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants