-
Notifications
You must be signed in to change notification settings - Fork 356
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: unlock mutex for experiment ResourcePool() [RM-152] #9119
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9119 +/- ##
=======================================
Coverage 46.16% 46.16%
=======================================
Files 1174 1174
Lines 145186 145186
Branches 2413 2413
=======================================
+ Hits 67026 67028 +2
+ Misses 77952 77950 -2
Partials 208 208
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
one crazy idea is to try and right a regex that catches
defer *.Unlock()
and put it in forbidigo
determined/master/.golangci.yml
Line 77 in 65339d2
forbidigo: |
@@ -32,8 +32,10 @@ import ( | |||
|
|||
var pAuthZ *mocks.ProjectAuthZ | |||
|
|||
const mockName = "mock" |
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.
note: had to add these changes to satisfy the linter, although my PR doesn't touch this
955339e
to
23aa3af
Compare
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.
suggested edit
Ticket
RM-152
Description
Unlock the internal experiment mutex in ResourcePool()
Test Plan
I added a unit test that checks that the function returns smoothly. Without my change, the test would timeout anyway/fail.
Checklist
docs/release-notes/
.See Release Note for details.