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

Add ManagedChannelContainer to JsonToGrpcGatewayFilterFactory. #3120 #3122

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add ManagedChannelContainer to JsonToGrpcGatewayFilterFactory. #3120 #3122

wants to merge 4 commits into from

Conversation

nsce9806q
Copy link

@nsce9806q nsce9806q commented Nov 5, 2023 •
edited

@spencergibb
Copy link
Member

@Albertoimpl or @abelsromero could one of you review?

@Albertoimpl
Copy link
Member

Using ManagedChannels is an excellent idea and should definitely improve the performance. Thanks a lot for the great contribution!

However, I was running the tests and now GRPCApplicationTests fails treating the exception when there is a runtime error.

@nsce9806q nsce9806q changed the title Add ManagedChannelContainer to AbstractGatewayFilterFactory. #3120 Add ManagedChannelContainer to JsonToGrpcGatewayFilterFactory. #3120 Mar 14, 2024
@nsce9806q
Refactor JsonToGrpcGatewayFilterFactory.GRPCResponseDecorator.createC…
4eda3e2 
…hannelChannel and modify for Checkstyle compliance
@nsce9806q
Copy link
Author

nsce9806q commented Mar 14, 2024 •
edited

@Albertoimpl Thank you for reviewing this PR.
I modified codes for checkstyle compliance.

GRPCApplicationTests passed for me like below:
image

can you give me more details to reproduce failure case?

@Albertoimpl
Copy link
Member

Everything looks great now after the updates @nsce9806q 👏🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Albertoimpl Albertoimpl Awaiting requested review from Albertoimpl

Assignees
No one assigned
Labels
waiting for feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nsce9806q @spencergibb @Albertoimpl @spring-cloud-issues