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.

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

Added option to configure the grpc histogram #1143

Merged
merged 2 commits into from
May 26, 2023
Merged

Added option to configure the grpc histogram #1143

merged 2 commits into from
May 26, 2023

Conversation

cristiancl25
Copy link
Contributor

No description provided.

@cristiancl25 cristiancl25 mentioned this pull request May 11, 2023
Closed
adriangonz
adriangonz reviewed May 17, 2023
View reviewed changes
Copy link
Contributor

@adriangonz adriangonz left a comment

Choose a reason for hiding this comment

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

Nice one @cristiancl25 !

Thanks for contributing this change. I think it's pretty reasonable - in fact, I was wondering whether it would make sense to remove the setting and always keep the enable_handling_time_histogram flag enabled?

mlserver/settings.py Outdated Show resolved Hide resolved
adriangonz
adriangonz approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@adriangonz adriangonz 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 the changes @cristiancl25 !

PR looks great now. Once tests are green, we can go ahead with this one. 🚀

@adriangonz
Copy link
Contributor

FYI that tests failure seems unrelated to this PR (should get fixed by #1164). Once that's merged, it may be good to rebase this PR and re-trigger the tests.

image

@adriangonz
Copy link
Contributor

Hey @cristiancl25 , the fix for the test issue above has already been merged. Could you rebase from master when you get a chance?

@cristiancl25
Copy link
Contributor Author

Done!

@adriangonz
Copy link
Contributor

Thanks for rebasing it! It should be ready to go now 👍

@adriangonz adriangonz merged commit 237df76 into SeldonIO:master May 26, 2023
26 checks passed
adriangonz pushed a commit that referenced this pull request May 26, 2023
@cristiancl25
Added option to configure the grpc histogram (#1143)
9722a3f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriangonz adriangonz adriangonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cristiancl25 @adriangonz