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.

Sign up for GitHub

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

Jump to bottom

Unable to generate FakeIngressServer in diego-logging-client #146

Closed
winkingturtle-vmw opened this issue Mar 24, 2023 · 3 comments
Closed

Unable to generate FakeIngressServer in diego-logging-client #146

winkingturtle-vmw opened this issue Mar 24, 2023 · 3 comments
Projects
DEPRECATED App Platform - Logging and...

Comments

@winkingturtle-vmw
Copy link

Hello,

We generate FakeIngressServer in diego-logging-client for testing our Send*Log methods in the library. Since the update protobuf PR #81 we see that there is a new private method mustEmbedUnimplementedIngressServer that is making our FakeIngressServer implementation no longer valid. Can you please advice how we are suppose to solve this problem so that we can continue generating a FakeIngressServer.

Context: We got here because we had to upgrade lager, which meant that we have to upgrade ginkgo to v2 everywhere in diego-release, which then meant we had to upgrade from go-loggregator v8 to v9, which then meant we had to upgrade diego-logging-client, which then meant we can no longer generate FakeIngressServer.

Do you have any suggestion for a workaround or other solution for this problem?
@winkingturtle-vmw
Copy link
Author

We were able to remain on v8 and get the subset of the tests that were failing to pass, but this remain to be an issue to upgrade to v9.

@ctlong ctlong moved this from Inbox to Reviewer Assigned in DEPRECATED App Platform - Logging and Metrics Apr 10, 2023
@ctlong ctlong moved this from Reviewer Assigned to Inbox in DEPRECATED App Platform - Logging and Metrics Jul 10, 2023
@ctlong
Copy link
Member

ctlong commented Jul 31, 2023

Hey @winkingturtle-vmw, sorry for the late response. Is this still a problem?

The reason for the change is a gRPC breaking change allowing generated files to be updated without requiring updates to server implementations. You can find this change discussed in more details here:

You should be able to resolve the diego-logging-client compilation error, and update to go-loggregator v9, by embedding UnimplementedIngressServer or UnsafeIngressServer in your FakeIngressServer struct. We recommend embedding the former.

@ctlong ctlong moved this from Inbox to Review in Progress in DEPRECATED App Platform - Logging and Metrics Jul 31, 2023
@winkingturtle-vmw
Copy link
Author

Brilliant, thanks for the tip. I am going to close this issue if that solution resolves the error and folks can still re-open if that doesn't work.

DEPRECATED App Platform - Logging and Metrics automation moved this from Review in Progress to Done Aug 1, 2023
Birdrock added a commit to Birdrock/diego-logging-client that referenced this issue May 21, 2024
@Birdrock @MarcPaquette
Embed UnimplementedIngressServer in FakeIngressServer struct
fc94561 
- A change in loggregator v9 requires that the
  UnimplementedIngressServer be embedded in FakeIngressServer as a
  result of a breaking change in gRPC.
- We used the recommended method from the go-loggregator team found in
  the following issue.
- cloudfoundry/go-loggregator#146
- The command to generate this file is: `counterfeiter -o testhelpers/ vendor/code.cloudfoundry.org/go-loggregator/v9/rpc/loggregator_v2/ingress_grpc.pb.go IngressServer`
- After generating the file, loggregator_v2.UnimplementedIngressServer will need to be embedded in the FakeIngressServer struct.

Signed-off-by: Marc Paquette <marc.paquette@broadcom.com>
Birdrock added a commit to Birdrock/diego-logging-client that referenced this issue May 21, 2024
@Birdrock @MarcPaquette
Embed UnimplementedIngressServer in FakeIngressServer struct
1f13dcd 
- A change in loggregator v9 requires that the
  UnimplementedIngressServer be embedded in FakeIngressServer as a
  result of a breaking change in gRPC.
- We used the recommended method from the go-loggregator team found in
  the following issue.
- cloudfoundry/go-loggregator#146
- The command to generate this file is: `counterfeiter -o testhelpers/ vendor/code.cloudfoundry.org/go-loggregator/v9/rpc/loggregator_v2/ingress_grpc.pb.go IngressServer`
- After generating the file, loggregator_v2.UnimplementedIngressServer will need to be embedded in the FakeIngressServer struct.

Signed-off-by: Marc Paquette <marc.paquette@broadcom.com>
@MarcPaquette MarcPaquette mentioned this issue May 21, 2024
Merged
1 task
MarcPaquette pushed a commit to cloudfoundry/diego-logging-client that referenced this issue May 21, 2024
@Birdrock
tas-runtime-bot
441b09b 
- A change in loggregator v9 requires that the
  UnimplementedIngressServer be embedded in FakeIngressServer as a
  result of a breaking change in gRPC.
- We used the recommended method from the go-loggregator team found in
  the following issue.
- cloudfoundry/go-loggregator#146
- The command to generate this file is: `counterfeiter -o testhelpers/ vendor/code.cloudfoundry.org/go-loggregator/v9/rpc/loggregator_v2/ingress_grpc.pb.go IngressServer`
- After generating the file, loggregator_v2.UnimplementedIngressServer will need to be embedded in the FakeIngressServer struct.

Signed-off-by: Marc Paquette <marc.paquette@broadcom.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
No open projects
DEPRECATED App Platform - Logging and...
Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ctlong @winkingturtle-vmw