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

Include idempotency level in UnaryServerInfo #1634

Conversation

keelerh
Copy link

@keelerh keelerh commented Oct 30, 2017

Adding idempotency level information to the UnaryServerInfo allows idempotency level to be used in server interceptors.

@thelinuxfoundation
Copy link

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
The Linux Foundation CLA GitHub bot

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@menghanl
Copy link
Contributor

Can you explain a bit more on the purpose of this change?
Also, IdempotencyLevel is never set to non-default value in the change.

@keelerh
Copy link
Author

keelerh commented Oct 30, 2017

Protoc now supports an idempotency_levelmethod option: https://github.com/google/protobuf/blob/9e745f7/src/google/protobuf/descriptor.proto#L630. The method option can be used to mark the idempotency of a gRPC method.

service Greeter {
  // Sends a greeting
  rpc SayHello (HelloRequest) returns (HelloReply) {
    option idempotency_level = NO_SIDE_EFFECTS;
  }
}

IdempotencyLevel would be set to a non-default value by explicitly setting the idempotency_level method option for a given service.

The purpose of the change is to include this idempotency_level information with the previously defined UnaryServerInfo such that it can be used at a later point, e.g. by server-side gRPC interceptors designed to ensure that services marked as idempotent do not suddenly become non-idempotent by calling a non-idempotent method.

@googlebot
Copy link

CLAs look good, thanks!

@keelerh
Copy link
Author

keelerh commented Oct 31, 2017

I signed it!

@dfawley
Copy link
Member

dfawley commented Nov 2, 2017

Thanks for the PR. We would prefer to implement this by making the entire proto file descriptor available to interceptors, which would include this information. (#1526) Unfortunately, that requires changes to generated code, which means it's blocked on us (this PR would have the same constraint). We hope to get to this early next quarter. Please let us know if you have any concerns with this.

@dfawley dfawley closed this Nov 2, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants