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

dependency: bump github.com/grpc-ecosystem/grpc-gateway/v2 from v2.7.0 to v2.17.1 #16357

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

liggitt
Copy link
Contributor

@liggitt liggitt commented Aug 2, 2023

This PR updates the grpc-gateway dependency to pick up grpc-ecosystem/grpc-gateway#3317

This removes the direct dependency on google.golang.org/genproto (which clears the way to clean up that dependency tree once google.golang.org/genproto/googleapis/rpc drops the reference to google.golang.org/genproto, see googleapis/go-genproto#1015 for details)

@serathius
Copy link
Member

FAIL: inconsistent versions for depencency: google.golang.org/genproto/googleapis/api
  - google.golang.org/genproto/googleapis/api@v0.0.0-20230525234035-dd9d682886f9 from: go.etcd.io/etcd/api/v3
  - google.golang.org/genproto/googleapis/api@v0.0.0-202307261[55](https://github.com/etcd-io/etcd/actions/runs/5740357922/job/15557997172?pr=16357#step:7:56)614-23370e0ffb3e from: go.etcd.io/etcd/server/v3

@liggitt
Copy link
Contributor Author

liggitt commented Aug 3, 2023

oops, fixed

@ahrtr
Copy link
Member

ahrtr commented Aug 3, 2023

etcd doesn't directly depend on github.com/grpc-ecosystem/grpc-gateway/v2. It's just an indirect dependency, which is introduced by go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc.

Usually we don't bump indirect dependencies directly [unless there is an urgent CVE or critical bug fix], because we don't know what compatibility issue it may cause. Instead, they (the indirect dependencies) will be automatically bumped each time when we bump the related direct dependencies. I believe this is also the reason why you do not bump grpc-gateway/v2 in Kubernetes directly.

Note that the grpc-gateway/v2 is already on the latest version v2.16.2 in otlptracegrpc's main branch.
So I would suggest to wait opentelemetry-go to release 1.16.1 or 1.17.0. Once it's released, then etcd will bump it, and the grpc-gateway will be automatically bumped by then.

References:

@ahrtr
Copy link
Member

ahrtr commented Sep 7, 2023

Eventually we will upgrade grpc-gateway to v2 in #16454. But just as I mentioned in #16454 (comment), I'd like to breakdown the big change, so I am happy to get this PR merged firstly. Could you please rebase this PR and bump grpc-gateway to latest version 2.17.1? Please feel free to let me know if you don't bandwidth to do it, than I can take care of it.

@liggitt
Copy link
Contributor Author

liggitt commented Sep 7, 2023

done

@ahrtr
Copy link
Member

ahrtr commented Sep 7, 2023

Could you rebase this PR although github doesn't show conflict? Your dev branch is far lag behind the head of main branch; sometimes github can't detect some conflict.

@liggitt
Copy link
Contributor Author

liggitt commented Sep 7, 2023

actually done this time :)

@ahrtr
Copy link
Member

ahrtr commented Sep 7, 2023

The workflow fails due to inconsistent google.golang.org/genproto/googleapis/api versions.

Please refer to 40c7f72

@liggitt liggitt force-pushed the fieldmask branch 2 times, most recently from 0bb99e6 to 90a2175 Compare September 7, 2023 12:45
…0 to v2.17.1

Signed-off-by: Jordan Liggitt <liggitt@google.com>
@liggitt
Copy link
Contributor Author

liggitt commented Sep 7, 2023

finally got it green

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

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 changed the title dependency: bump github.com/grpc-ecosystem/grpc-gateway/v2 from v2.7.0 to v2.16.2 dependency: bump github.com/grpc-ecosystem/grpc-gateway/v2 from v2.7.0 to v2.17.1 Sep 7, 2023
@ahrtr ahrtr merged commit 7192b6e into etcd-io:main Sep 7, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants