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

gRPC server reflection support #5518

Merged
merged 13 commits into from
Dec 14, 2022

Conversation

jackkav
Copy link
Contributor

@jackkav jackkav commented Dec 13, 2022

changelog(Improvements): Added gRPC Server Reflection support

Highlights:

  • added a dropdown element to fetch methods from reflection
  • will ignore services when protoloader fails to parse

image

Future work:

  • provide UX for unreachable server
  • replace override with bumps
  • solve duplicate errors on services protoloader can't parse
  • support loading multiple proto files from a given directory when not using reflection

@jackkav jackkav requested a review from a team December 13, 2022 23:05
@jackkav jackkav self-assigned this Dec 13, 2022
@jackkav jackkav added the A-grpc Area: GRPC / GRPC Protocol label Dec 13, 2022
@jackkav jackkav force-pushed the refactor/grpc-towards-reflection branch from b286c50 to cbb1837 Compare December 13, 2022 23:32
@jackkav jackkav marked this pull request as ready for review December 14, 2022 00:08
@@ -211,5 +212,8 @@
},
"dev": {
"dev-server-port": 3334
},
"overrides": {
"protobufjs": "7.1.2"
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth writing a comment about the issue without the override with the libs import 6.9.x

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 am planning to remove the overrides, I've attempted to bump protobuf.js in a fork of grpc-reflection-js but no luck yet. I'll add it to the todo, may come in a follow up PR.

Copy link
Member

@filfreire filfreire left a comment

Choose a reason for hiding this comment

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

const packageDefinition = protoLoader.loadFileDescriptorSetFromObject(descriptorMessage, {});
return getMethodsFromPackageDefinition(packageDefinition);
} catch (e) {
console.error(e);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could surface errors to the UI. For example - if the user is not able to connect to the server, we can see an error on console, but the UI silently fails.

Screenshot 2022-12-14 at 10 02 39

Copy link
Contributor

@gatzjames gatzjames left a comment

Choose a reason for hiding this comment

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

🚀

@jackkav jackkav force-pushed the refactor/grpc-towards-reflection branch from 784b1c4 to 690cd2a Compare December 14, 2022 11:47
@jackkav jackkav force-pushed the refactor/grpc-towards-reflection branch from 690cd2a to 0aeceba Compare December 14, 2022 12:14
@jackkav jackkav enabled auto-merge (squash) December 14, 2022 12:20
@jackkav jackkav merged commit 1bb9607 into Kong:develop Dec 14, 2022
@jackkav jackkav deleted the refactor/grpc-towards-reflection branch December 14, 2022 12:27
pavkout pushed a commit to pavkout/insomnia that referenced this pull request Jan 18, 2023
* eliminate grpc paths

* add fake reflection ux

* add grpc packages

* basic ux

* first working pass

* package lock

* reset selected protofile

* fix types

* ssl

* improve naming

* remove deprecated grpc url parse

* remove broken test

* replace grpc proto PR test with reflection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-grpc Area: GRPC / GRPC Protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants