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

Consider a request builder and response parser design #17

Open
ryanschneider opened this issue Mar 5, 2020 · 1 comment
Open

Consider a request builder and response parser design #17

ryanschneider opened this issue Mar 5, 2020 · 1 comment
Labels
good first issue Good for newcomers issue-type/tech-debt This includes any maintenance, testing or refactoring that we need to do. priority/? unknown priority size/? unknown time estimate team/backend Work for a backend engineer.

Comments

@ryanschneider
Copy link
Contributor

Thinking about #7 more, and my general unhappiness with the verbosity of the node.Client interface, it might make more sense to remove all the RPC-specific methods from that interface, and instead offer them as some sort of request builder and response parser pair.

So instead of:

block, err := client.BlockByHash(ctx, hash, true, node.WithRequestID(jsonrpc.ID{Str: "foo"}))

One would instead use:

block, err := pkg.NewRequest().BlockByHash(hash, true).WithStringID("foo").Send(client).Response().AsBlock()

Though not sure where said builders would live (e.g. what the actual name of pkg would be).

This pattern is used in a couple places to varying effectiveness, namely go-redis and the golang AWS SDK v2. Anyways, just a rough idea for now, but the main advantage is that it separates the definition of new RPCs from node.Client, which just needs to implement .Request and .Subscribe which makes implementing new virtual clients and or mocks much cleaner.

@ryanschneider ryanschneider added size/? unknown time estimate team/backend Work for a backend engineer. enhancement good first issue Good for newcomers labels Apr 21, 2020
@ryanschneider
Copy link
Contributor Author

This would also make supporting batch requests doable, as the Client would just need to implement an additional .BatchRequest method, and the []jsonrpc.Request could be built using the builder.

@frankreed frankreed added issue-type/tech-debt This includes any maintenance, testing or refactoring that we need to do. and removed enhancement labels Apr 29, 2020
@frankreed frankreed added the priority/? unknown priority label May 22, 2020
@skmgoldin skmgoldin removed their assignment Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers issue-type/tech-debt This includes any maintenance, testing or refactoring that we need to do. priority/? unknown priority size/? unknown time estimate team/backend Work for a backend engineer.
Projects
None yet
Development

No branches or pull requests

3 participants