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

fix: logger.warn instead of ISE on transcoding options for non-unary gRPC #5829

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

Dogacel
Copy link
Contributor

@Dogacel Dogacel commented Jul 25, 2024

Motivation:

Modifications:

  • Warn and skip the method instead of preventing server startup.

Result:

@@ -237,6 +237,12 @@ service HttpJsonTranscodingTestService {
post: "/v1/arbitrary_wrapped"
};
}

rpc EchoBidirectionalStream(stream Message) returns (stream Message) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have verified the tests used to fail before the fix on HttpTranscodingService.java.

In my opinion we shouldn't be so harsh about unsupported features and punish developers.

@ikhoon ikhoon added the defect label Jul 26, 2024
@ikhoon ikhoon added this to the 1.29.3 milestone Jul 26, 2024
Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks, @Dogacel!

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks, @Dogacel!

@ikhoon ikhoon merged commit e2a30e7 into line:main Jul 26, 2024
14 of 15 checks passed
ikhoon added a commit to ikhoon/armeria that referenced this pull request Jul 26, 2024
ikhoon added a commit that referenced this pull request Jul 26, 2024
ikhoon added a commit that referenced this pull request Jul 26, 2024
ikhoon pushed a commit that referenced this pull request Jul 26, 2024
…gRPC (#5829)

Motivation:

- #5828
- When an rpc method has transcoding options, the server won't start up.

Modifications:

- Warn and skip the method instead of preventing server startup.

Result:

- Closes #5828
- Server can start if a gRPC non-unary rpc is configured to have
transcoding options.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Soft-fail on error "Only unary methods can be configured with an HTTP/JSON endpoint"
4 participants