-
Notifications
You must be signed in to change notification settings - Fork 115
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
go/keymanager/api: Fix key manager client #6078
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for oasisprotocol-oasis-core canceled.
|
@@ -12,8 +12,10 @@ import ( | |||
|
|||
var ( | |||
// serviceName is the gRPC service name. | |||
serviceName = cmnGrpc.NewServiceName("KeyManager") | |||
serviceName = cmnGrpc.NewServiceName("KeyManager.Secrets") |
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.
Is this too breaking? Not sure who even calls this endpoint, since all nodes run consensus 🤔
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.
Should be fine. Maybe expose both for a few versions and use the new one in clients?
5210628
to
c354245
Compare
c354245
to
e22cea4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6078 +/- ##
==========================================
+ Coverage 64.86% 65.26% +0.40%
==========================================
Files 636 636
Lines 64641 64729 +88
==========================================
+ Hits 41929 42248 +319
+ Misses 17823 17586 -237
- Partials 4889 4895 +6 ☔ View full report in Codecov by Sentry. |
Moving key manager backend from
ServicesBackend
toClientBackend
.I also refactored clients, trying to follow these guidelines: