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

Update Go protobuf package to new version #21515

Closed
damccorm opened this issue Jun 5, 2022 · 8 comments · Fixed by #32045
Closed

Update Go protobuf package to new version #21515

damccorm opened this issue Jun 5, 2022 · 8 comments · Fixed by #32045

Comments

@damccorm
Copy link
Contributor

damccorm commented Jun 5, 2022

The original Go protobuf package (github.com/golang/protobuf/proto) is deprecated and should be replaced with the new version (google.golang.org/protobuf/proto)

Imported from Jira BEAM-14242. Original Jira may contain additional context.
Reported by: jrmccluskey.
Subtask of issue #21513

@imvtsl
Copy link
Contributor

imvtsl commented Jul 25, 2024

.take-issue

@imvtsl
Copy link
Contributor

imvtsl commented Jul 30, 2024

I am now working on this. Thanks.

@imvtsl
Copy link
Contributor

imvtsl commented Jul 31, 2024

Hi @damccorm,

I raised this draft PR. This is my first contribution to this repository.
I am able to update all but one file. I would require help with upgrading sdks/go/pkg/beam/create_test.go. Can I get some help with it?

Thanks.

@damccorm
Copy link
Contributor Author

damccorm commented Aug 2, 2024

@lostluck or @damondouglas could one of you please take a look and see if you can help here?

@lostluck
Copy link
Contributor

lostluck commented Aug 2, 2024

I'll take a look! Thank you very much!

@imvtsl
Copy link
Contributor

imvtsl commented Aug 2, 2024

@lostluck @damccorm
Thanks for the response.
I have a couple of approaches to try out. I will try to do them this weekend and I will share my conclusions. Thank you once again!

@lostluck
Copy link
Contributor

lostluck commented Aug 2, 2024

The PR is pretty great as is TBH!

So the clarify the goal of this issue:

The intent is that most of beam isn't relying on the deprecated package to function. So the changes you've made a great thus far.

However, we do want to ensure that if users are somehow still relying on v1 protocol buffer types that they're using, that they continue to function.

This means we need to keep the code in beam/create_test.go and the protov1 handling code in beam/coder.go with some very small changes.

I'll add comments to the PR directly.

@imvtsl
Copy link
Contributor

imvtsl commented Aug 3, 2024

Thank you for the comments. I was going in a different direction. Your clarifications saved me a lot of time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants