-
Notifications
You must be signed in to change notification settings - Fork 211
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: make set_active_gateway concurrency safe #4162
Conversation
67aa607
to
b4227a7
Compare
i am conflicted with return Error and using lock error might help you find concurrent calls with set_active_gateway, but lock feels more correct semantically |
Why not get rid of |
Why not auto-commit, if you want to avoid the error case? I don't think it makes sense though, just as the lock version. In order of preference:
|
we still have to keep active gateway in fedimint-cli because of devimint |
Relevant discussion in #3844 |
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.
Propagating the error up is IMO the best short term solution
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4162 +/- ##
==========================================
+ Coverage 58.01% 58.05% +0.03%
==========================================
Files 192 192
Lines 42990 42990
==========================================
+ Hits 24941 24957 +16
+ Misses 18049 18033 -16 ☔ View full report in Codecov by Sentry. |
fixes #4110
just put a lock around it for nowreturn error on concurrent calls