Skip to content

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
Member

@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
@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.

5 participants