Skip to content
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

Deprecate Envoy API v2 primitives in the service proto for Request Source. #589

Merged
merged 5 commits into from
Dec 8, 2020
Merged

Deprecate Envoy API v2 primitives in the service proto for Request Source. #589

merged 5 commits into from
Dec 8, 2020

Conversation

mum4k
Copy link
Collaborator

@mum4k mum4k commented Dec 4, 2020

Converting the current field into a oneof and marking the v2 primitive as deprecated. The old field retains its functionality for backward compatibility.

Also:

  • sorting imports in api/request_source/service.proto.
  • migrating our dummy request source to using the v3 primitive.
  • improving error message emitted on test failures in test/request_stream_grpc_client_test.cc, the current message just dumps byte representation of the two compared header objects.

Fixes #580.

Signed-off-by: Jakub Sobon mumak@google.com

mum4k added 3 commits December 4, 2020 14:42
Signed-off-by: Jakub Sobon <mumak@google.com>
Signed-off-by: Jakub Sobon <mumak@google.com>
Signed-off-by: Jakub Sobon <mumak@google.com>
@mum4k mum4k requested review from oschaaf and dubious90 December 4, 2020 20:03
@mum4k mum4k added the waiting-for-review A PR waiting for a review. label Dec 4, 2020
@mum4k
Copy link
Collaborator Author

mum4k commented Dec 4, 2020

@dubious90 please review and assign back to me once done.

dubious90
dubious90 previously approved these changes Dec 4, 2020
Copy link
Contributor

@dubious90 dubious90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good modulo the one copy-paste error

test/request_stream_grpc_client_test.cc Outdated Show resolved Hide resolved
oschaaf
oschaaf previously approved these changes Dec 5, 2020
Copy link
Member

@oschaaf oschaaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but one thought I wanted to float for consideration. LGTM if you lean towards keeping this as is.

api/request_source/service.proto Show resolved Hide resolved
@mum4k mum4k dismissed stale reviews from oschaaf and dubious90 via 32d5123 December 8, 2020 04:52
@mum4k
Copy link
Collaborator Author

mum4k commented Dec 8, 2020

@oschaaf @dubious90 Could one of you take another look and re-approve after addressing the comments? Thank you.

@mum4k mum4k merged commit 602eead into envoyproxy:master Dec 8, 2020
@mum4k mum4k deleted the request-source-api-v2-to-v3 branch December 8, 2020 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-review A PR waiting for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Nighthawk's use of Envoy APIs to v3
3 participants