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 interface reflection. #6722

Merged
merged 18 commits into from
Aug 17, 2020
Merged

gRPC interface reflection. #6722

merged 18 commits into from
Aug 17, 2020

Conversation

aaronc
Copy link
Member

@aaronc aaronc commented Jul 14, 2020

Description

ref: #5921

This PR adds a new gRPC service to do interface registry reflection. Specifically, it adds 2 RPC endpoints:

  • ListAllInterfaces: list all the interfaces registered in the registry.
  • ListImplementations: given an interface name, list all its implementations.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Jul 14, 2020

Codecov Report

Merging #6722 into master will decrease coverage by 0.01%.
The diff coverage is 45.23%.

@@            Coverage Diff             @@
##           master    #6722      +/-   ##
==========================================
- Coverage   54.68%   54.66%   -0.02%     
==========================================
  Files         540      541       +1     
  Lines       36856    36887      +31     
==========================================
+ Hits        20153    20165      +12     
- Misses      15060    15077      +17     
- Partials     1643     1645       +2     

@aaronc aaronc mentioned this pull request Jul 15, 2020
43 tasks
Copy link
Collaborator

@odeke-em odeke-em 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 this change @aaronc, just a drive by code review from me.

client/grpc/reflection/reflection.go Outdated Show resolved Hide resolved
client/grpc/reflection/reflection.go Outdated Show resolved Hide resolved
codec/types/interface_registry.go Outdated Show resolved Hide resolved
client/grpc/reflection/reflection.go Outdated Show resolved Hide resolved
@alexanderbez
Copy link
Contributor

Can you add a bit more to the PR body @aaronc? Thanks for listing the ref, I just wanna be sure of what this changeset includes and why :)

@amaury1093
Copy link
Contributor

@alexanderbez I just updated the PR desc answering the "what" question.

Concerning the "why", tbh I'm not 100% sure, I see it as a nice-to-have, same as the gRPC reflection itself. Aaron might have a clearer idea.

@amaury1093 amaury1093 marked this pull request as ready for review August 17, 2020 16:42
@odeke-em
Copy link
Collaborator

odeke-em commented Aug 17, 2020 via email

client/grpc/reflection/reflection.go Show resolved Hide resolved
codec/types/interface_registry.go Outdated Show resolved Hide resolved
codec/types/interface_registry.go Outdated Show resolved Hide resolved
amaury1093 and others added 4 commits August 17, 2020 18:52
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

🎉

@alexanderbez alexanderbez added A:automerge Automatically merge PR once all prerequisites pass. C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. labels Aug 17, 2020
@mergify mergify bot merged commit 3f81c0a into master Aug 17, 2020
@mergify mergify bot deleted the aaronc/5921-proto-reflection branch August 17, 2020 17:02
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
* WIP on gRPC interface reflection.

* Update docs in proto

* Add tests

* Add test

* Add route inside router

* Address nits

* ListInterfaces -> ListAllInterfaces

* Fix proto lint

* Remove stray println

* Update proto/cosmos/base/reflection/v1beta1/reflection.proto

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

* Update codec/types/interface_registry.go

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

* Update codec/types/interface_registry.go

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

* Add godoc

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C: gRPC Issues and PRs related to the gRPC service and HTTP gateway.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants