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

feat(rpc): Add client wrapper #1195

Merged
merged 6 commits into from
Nov 1, 2022

Conversation

distractedm1nd
Copy link
Member

@distractedm1nd distractedm1nd commented Oct 4, 2022

Based on #1175
Closes #1185
Closes #1186

Also adds the basis for the new rpc tests:

  1. Making sure all methods we expose over the API actually exist
  2. Making sure all the types we return can be marshalled in and out of json
  3. RPC specific special behaviors, like authentication

@distractedm1nd distractedm1nd self-assigned this Oct 4, 2022
@distractedm1nd distractedm1nd added kind:feat Attached to feature PRs area:api Related to celestia-node API labels Oct 4, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1195 (b7ba257) into main (21aed31) will decrease coverage by 5.20%.
The diff coverage is 64.79%.

@@            Coverage Diff             @@
##             main    #1195      +/-   ##
==========================================
- Coverage   60.25%   55.04%   -5.21%     
==========================================
  Files         161      163       +2     
  Lines        9736     9575     -161     
==========================================
- Hits         5866     5271     -595     
- Misses       3337     3777     +440     
+ Partials      533      527       -6     
Impacted Files Coverage Δ
api/gateway/availability.go 0.00% <0.00%> (ø)
api/gateway/config.go 0.00% <0.00%> (ø)
api/gateway/das.go 0.00% <ø> (ø)
api/gateway/endpoints.go 0.00% <ø> (ø)
api/gateway/handler.go 0.00% <0.00%> (ø)
api/gateway/header.go 0.00% <ø> (ø)
api/gateway/middleware.go 29.41% <0.00%> (ø)
api/gateway/server.go 68.88% <ø> (ø)
api/gateway/share.go 0.00% <ø> (ø)
api/gateway/state.go 0.00% <ø> (ø)
... and 102 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

pushing initial review:
should probs be a api/server and api/client (unless that breaks deps)

nodebuilder/rpc_test.go Outdated Show resolved Hide resolved
api/rpc/client/client.go Outdated Show resolved Hide resolved
renaynay
renaynay previously approved these changes Oct 31, 2022
api/rpc/client/client.go Outdated Show resolved Hide resolved
nodebuilder/das/service.go Outdated Show resolved Hide resolved
nodebuilder/header/service.go Outdated Show resolved Hide resolved
nodebuilder/rpc/rpc.go Show resolved Hide resolved
nodebuilder/testing.go Show resolved Hide resolved
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

t.Cleanup closer and contexts in tests

api/rpc/client/client.go Outdated Show resolved Hide resolved
nodebuilder/das/service.go Show resolved Hide resolved
nodebuilder/header/service.go Outdated Show resolved Hide resolved
nodebuilder/testing.go Show resolved Hide resolved
nodebuilder/rpc_test.go Outdated Show resolved Hide resolved
nodebuilder/rpc_test.go Outdated Show resolved Hide resolved
nodebuilder/rpc_test.go Outdated Show resolved Hide resolved
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Approving, unit test failure is known flake that will be resolved with #1286

@distractedm1nd distractedm1nd merged commit 9c996a7 into celestiaorg:main Nov 1, 2022
@distractedm1nd distractedm1nd deleted the client-wrapper branch November 1, 2022 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Related to celestia-node API kind:feat Attached to feature PRs
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add fraud service to rpc Add client wrapper
5 participants