regen-network / cosmos-modules

Other
10 stars 2 forks source link

Group querier service definition #63

Open aaronc opened 4 years ago

aaronc commented 4 years ago

This provides the protobuf service definition for supported queries, providng the specification for #42.

The specification for how this is linked up with the querier is still TBD, but needs to be done for Cosmos SDK soon anyway so I will get to that next. Basically the starting point would be to implement the QueryServer interface either on the Keeper directly or with the Keeper as the sole member. And we should think of a relatively simple pattern for retrieving sdk.Context from context.Context (or even just a readonly store as that's all querier's should have access to ideally).

aaronc commented 4 years ago

So in option B you would create pagination request/response types for each model type? i.e. ProposalsRequest/Response. I would prefer that. Could you maybe just share how that would work as a protobuf code snippet?

And l don't think we need to worry about proofs right now btw

alpe commented 4 years ago

I have some problems to understand the use of protobuf for queries. The ABCI defines for queries:

Data ([]byte): Raw query bytes. Can be used with or in lieu of Path. Path (string): Path of request, like an HTTP GET path. Can be used with or in liue of Data

So the protobuf could go into the Data field of queries and Value of responses instead of json. But how does this fit with the RPC service definitions?

I have modified #42 with the cursor implementation. See https://github.com/regen-network/cosmos-modules/pull/50/files#diff-ed6ce47e449d64aeaf419a56cdaca5e5R47-R96 how this would look. Clients would not have to construct the cursors themselfs as they are returned in the response payload.

This are the data types used. It would not make sense to copy them to protobuf directly as some are read from the abci Path field

Request

Response

type Model struct { Key []byte json:"key", yaml:"key" Value json.RawMessage json:"value", yaml:"value" }

aaronc commented 4 years ago

I have some problems to understand the use of protobuf for queries. The ABCI defines for queries:

Data ([]byte): Raw query bytes. Can be used with or in lieu of Path. Path (string): Path of request, like an HTTP GET path. Can be used with or in liue of Data

So the protobuf could go into the Data field of queries and Value of responses instead of json. But how does this fit with the RPC service definitions?

For an RPC service call there are two things - method name and request payload, those would fit into ABCI Path and Data respectively. Custom queriers in Cosmos SDK (type Querier = func(ctx Context, path []string, req abci.RequestQuery) (res []byte, err Error)) just return []byte or an error. So there's a pretty straightforward way to map protobuf service definition onto this. I intend to spike out a POC of this soon.

aaronc commented 4 years ago

Also I think queries directly against the table store like you are suggesting are a good idea but my thought is that they are complementary solutions. Would there be a way to generate a swagger schema just from the metadata in the orm?

For me encapsulating custom queries using the service definition to seems like a good approach because it allows existing protobuf definitions to be used. Some Cosmos SDK modules may not be using the ORM package (yet) so I would like a protobuf way for supporting queries on them.

aaronc commented 4 years ago

@alpe see the changes to querier.go and the AppModule to see how I've wired up the querier to the an implementation of the QueryServer interface that was generated by the protobuf grpc plugin. Basically this uses the grpc infrastructure to route queries via path to rpc handler methods.

aaronc commented 4 years ago

@alpe see https://github.com/cosmos/cosmos-sdk/pull/5894 for reference on how gRPC queries will be done in Cosmos SDK.