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

grpc-loader: Expose method options #2711

Merged
merged 16 commits into from
Apr 9, 2024
Merged

grpc-loader: Expose method options #2711

merged 16 commits into from
Apr 9, 2024

Conversation

n0v1
Copy link
Contributor

@n0v1 n0v1 commented Apr 8, 2024

This PR adds support for MethodOptions to proto-loader.

It is a continuation of #2230 (originally written by @hiepthai) which tries to address the comments of @murgatroid99.

hiepthai and others added 15 commits April 5, 2024 15:32
@hiepthai @n0v1
grpc-loader: add method options in MethodDefinition
c7d4566
@hiepthai @n0v1
grpc-loader: update MethodDefinition interface
c8b5e05
@hiepthai @n0v1
grpc-loader: map method options and add MethodOptions interface
fc9db76
@hiepthai @n0v1
grpc-loader: add method options test
dfb10f9
@hiepthai @n0v1
grpc-loader: export interfaces and fix minor issue
ae33e87
@hiepthai @n0v1
grpc-loader: update & correct interface types
68ea1f4
@hiepthai @n0v1
grpc-loader: cleanup MethodOptions interfaces
d335428
@hiepthai @n0v1
grpc-loader: update interfaces & add new test
4c185fa
@n0v1
predefined options should always be returned with their default value
260966c
@n0v1
narrow down type of idempotency_level
5bd37a9
@n0v1
make uninterpreted_option an array
99e8a06
@n0v1
adjust implementation to always return default options
27d6087
@n0v1
add test case for partially set options
1e37caa
@n0v1
use single quotes consistently
6e441a8
@n0v1
return uninterpreted options as an array
e6ac1a4
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Apr 8, 2024 •
edited

CLA Signed

The committers listed above are authorized under a signed CLA.

@n0v1 n0v1 mentioned this pull request Apr 8, 2024
Closed
murgatroid99
murgatroid99 requested changes Apr 8, 2024
View reviewed changes
Copy link
Member

@murgatroid99 murgatroid99 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. This addresses a lot of the concerns I had with the previous attempt. You will need to fix the CLA issues before I can accept it.

murgatroid99
murgatroid99 approved these changes Apr 9, 2024
View reviewed changes
Copy link
Member

@murgatroid99 murgatroid99 left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this up. It looks good now.

@murgatroid99 murgatroid99 merged commit 5674498 into grpc:master Apr 9, 2024
4 of 5 checks passed
@n0v1 n0v1 deleted the feat/method-options branch April 10, 2024 07:23
@murgatroid99 murgatroid99 mentioned this pull request Apr 10, 2024
Closed
@EitanGayor EitanGayor mentioned this pull request May 22, 2024
Open
@j-mendez j-mendez mentioned this pull request May 22, 2024
Open
This was referenced May 23, 2024
Open
Open
@j-mendez j-mendez mentioned this pull request May 25, 2024
Open
This was referenced May 31, 2024
Open
Open
Open
@Suryaveersingh8055 Suryaveersingh8055 mentioned this pull request Jun 5, 2024
Open
@EitanGayor EitanGayor mentioned this pull request Jun 6, 2024
Open
@n0v1 n0v1 mentioned this pull request Jun 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@murgatroid99 murgatroid99 murgatroid99 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@n0v1 @murgatroid99 @kokoro-team @hiepthai