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

Remove deprecated grpc methods #197

Merged
merged 6 commits into from
Jan 7, 2025

Conversation

abjeni
Copy link
Collaborator

@abjeni abjeni commented Jan 7, 2025

Removing deprecated grpc methods and using the new NewClient method instead of DialContext.
This also removes the gorums.WithDialTimeout method.
This means that you cannot know if the grpc server is not responding before you use any rpc call.

  • list of removed methods
  1. grpc.DialContext
  2. grpc.WithBlock
  3. grpc.grpc.WithReturnConnectionError

Fixes #196

Copy link
Member

@meling meling left a comment

Choose a reason for hiding this comment

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

Great job!

@meling
Copy link
Member

meling commented Jan 7, 2025

For the record, I would like to get this one into master before the multiparty PR.

@abjeni Could you check how bad the conflicts would be if we merge this one into master before merging the Multiparty PR?

I'm sure there is a smart way to do it, but one naive way would be to do another clone of the repo to a tmp folder, and checkout this branch, and then run:

git switch Salhus/issue196/remove-deprecated-methods
git merge multiparty

Alternatively, the other way around.

(If you are confident with git, you can do it on your local working copy and roll back.)

@abjeni
Copy link
Collaborator Author

abjeni commented Jan 7, 2025

I used the naive method and only got one merge conflict where we both modified the managerOptions struct in file opts.go. I also did not notice any deprecated grpc methods.

@meling
Copy link
Member

meling commented Jan 7, 2025

Excellent, one merge conflict is not a problem...

@meling meling merged commit b8287c2 into master Jan 7, 2025
3 checks passed
@meling meling deleted the Salhus/issue196/remove-deprecated-methods branch January 7, 2025 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore: remove use of deprecated APIs no longer recommended with newer gRPC versions
2 participants