-
Notifications
You must be signed in to change notification settings - Fork 357
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
fix: do not modify cached GetAgentsResponse #9569
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9569 +/- ##
==========================================
- Coverage 49.80% 49.80% -0.01%
==========================================
Files 1247 1247
Lines 162241 162249 +8
Branches 2888 2887 -1
==========================================
+ Hits 80803 80806 +3
- Misses 81266 81271 +5
Partials 172 172
Flags with carried forward coverage won't be shown. Click here to find out more.
|
return k.jobsService.GetAgents(), nil | ||
resp := k.jobsService.GetAgents() | ||
// Ensure cached response is not inadvertently modified. | ||
return rm.CopyGetAgentsResponse(resp) |
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.
probably is safer to do this copy on jobsService.getAgents
in case someone down the line calls this within the jobs service
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.
By that do you mean, copy the response before returning from jobsService.GetAgents()
?
I had that originally, but the function signature didn't return an error. Thinking about it, there's no reason I can't update the jobsService function to return an error. :)
(cherry picked from commit 8fbed0e)
Ticket
RM-350
Description
The Kubernetes resource manager caches the response to a
GetAgents
request. Return a deep copy of the cached response to prevent unintentionally modifying the cached response.Test Plan
Checklist
docs/release-notes/
See Release Note for details.