-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
etcdserver: improve Lease http path naming for gRPC gateway #9450
Conversation
I have no excuse to not add curl leases tests here ... |
Codecov Report
@@ Coverage Diff @@
## master #9450 +/- ##
=========================================
Coverage ? 72.23%
=========================================
Files ? 364
Lines ? 30887
Branches ? 0
=========================================
Hits ? 22310
Misses ? 6940
Partials ? 1637 Continue to review full report at Codecov.
|
@@ -615,17 +615,7 @@ | |||
"Lease" | |||
], | |||
"summary": "LeaseLeases lists all existing leases.", | |||
"operationId": "LeaseLeases", | |||
"parameters": [ |
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.
The parameters blocks such as this getting deleted in the auto-generated file doesn't seem right. Maybe a problem in the way "additional-bindings" work?
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.
Yeah something is amiss, it works but not consistently. Possible upstream bug, but not sure.
@hexfusion Can we also update changelogs? |
@akashbaid, I find the below to be stable and produce proper Swagger. WIll update PR and tests once #9460 gets merged.
|
That's great! Thanks for looking into it. |
@gyuho updated release notes and tweaked lease tests for coverage. Should be good on greens. |
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. thanks @hexfusion !
This PR looks to normalize some of the lease api paths for the gRPC gateway. Using additional_bindings supported by grpc-gateway we can accept both old and new paths. Current plan is to support in 3.4 and deprecate all additional_bindings in 3.5. I will work on notifying existing clients of the change as well.
fixes #9430
/cc @akashbaid