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

Upgrade grpc-gateway from v1 to v2 #16595

Merged
merged 7 commits into from
Sep 19, 2023
Merged

Upgrade grpc-gateway from v1 to v2 #16595

merged 7 commits into from
Sep 19, 2023

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Sep 13, 2023

  • This PR is based on Upgrade grpc-gateway from v1 to v2 #16454.
  • This PR is co-authored by @ahrtr and. @fuweid
  • We are still depending on the gogo/protobuf. We only bump grpc-gateway from v1 to v2 in this PR, so as to avoid any potential compatibility issues between gogo/protobuf and protobuf-go v2

cc @fuweid @jmhbnz @mitake @ptabor @serathius @spzala @wenjiaswe @dims @liggitt

To clarify, although this PR looks huge, but actually majority source code are generated by protoc plugins.

@ahrtr ahrtr added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. dependencies Pull requests that update a dependency file labels Sep 13, 2023
@ahrtr
Copy link
Member Author

ahrtr commented Sep 13, 2023

cc @johanbrandhorst as well. Please kindly let's know if you have any concern.

Copy link

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

LGTM


trap cleanup_googleapi EXIT

# TODO(ahrtr): use buf (https://github.com/bufbuild/buf) to manage the protobuf dependencies?

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr ahrtr force-pushed the gw_20230913 branch 2 times, most recently from f4ebc61 to dc1172a Compare September 14, 2023 08:56
@ahrtr
Copy link
Member Author

ahrtr commented Sep 15, 2023

This PR is trying to resolve an important task tracked in 3.6 roadmap. Ping all etcd maintainers to take a look, thx.

@ahrtr
Copy link
Member Author

ahrtr commented Sep 18, 2023

@tao12345666333 @CyberDem0n & anyone else, Could you please also verify this PR on your project, so as to make sure it doesn't break anything? Thanks.

fuweid and others added 6 commits September 18, 2023 11:22
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Benjamin Wang <wachao@vmware.com>
Run ./scripts/genproto.sh

Signed-off-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
…s to v2 messages

Signed-off-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
…2.17.1 for all modules

Signed-off-by: Benjamin Wang <wachao@vmware.com>
1. Manually updated go source file to remove the usage of v1 grpc-gateway;
2. Execute ./scripts/fix.sh

Signed-off-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Benjamin Wang <wachao@vmware.com>
…script or code

Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr
Copy link
Member Author

ahrtr commented Sep 19, 2023

With the approval of 1 maintainer and 3 members/contributors (including a grpc-gateway expert), and high level support from the K8s core members (@liggitt, @dims ), merging this PR...

@ahrtr ahrtr merged commit c70ac64 into etcd-io:main Sep 19, 2023
27 checks passed
@ahrtr
Copy link
Member Author

ahrtr commented Sep 19, 2023

thx all. @fuweid @johanbrandhorst @serathius @tao12345666333

Especially a big THANK YOU to @fuweid, who demonstrated deep understanding on both etcd and grpc-gateway/protobuf.

@ahrtr
Copy link
Member Author

ahrtr commented Sep 19, 2023

thx @liggitt and @dims as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Development

Successfully merging this pull request may close these issues.

None yet

5 participants