-
Notifications
You must be signed in to change notification settings - Fork 215
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
[YUNIKORN-2515] Add property event.RESTResponseSize to the batch event handler #886
Conversation
I think |
Created #887 to fix the test case. Will rebase this PR as soon as that one gets merged. |
I have another idea of implementing this new property.
yunikorn-core/pkg/events/event_ringbuffer.go Line 104 in d46f216
|
Hm... I'd rather keep this separate. This property concerns the REST output, I think it's better to handle it separately from the event system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pbacsko thanks for updated PR. overall LGTM. one last comment :)
pkg/webservice/handlers.go
Outdated
configs.AddConfigMapCallback("rest-response-size", func() { | ||
newSize := configs.DefaultRESTResponseSize | ||
configMap := configs.GetConfigMap() | ||
if value, ok := configMap[configs.CMRESTResponseSize]; ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about reusing common.GetConfigurationUint
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #886 +/- ##
==========================================
+ Coverage 77.43% 77.69% +0.25%
==========================================
Files 97 97
Lines 12132 12119 -13
==========================================
+ Hits 9395 9416 +21
+ Misses 2410 2379 -31
+ Partials 327 324 -3 ☔ View full report in Codecov by Sentry. |
What is this PR for?
Update the code with the property
event.RESTResponseSize
as per the design document.What type of PR is it?
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2515
How should this be tested?
Screenshots (if appropriate)
Questions: