spacemeshos / api

Protobuf implementation of the Spacemesh API
MIT License
15 stars 9 forks source link

Handle buf warnings for streaming methods http rules #150

Open avive opened 3 years ago

avive commented 3 years ago

Currently, there are no http rules for the streaming methods. These rules are used to generate the gateway json api. A bit of research is needed hre- can thegateway json api support Spacemesh streaming grpc methods? if yes - update the rules to include the streaming methods. If not - remove the build warnings about missing streaming methods. @daonb @lrettig

lrettig commented 3 years ago

The json gateway should be able to support the streaming endpoints. But I don't think we've ever tested this. I'm disinclined to add them since we should generally discourage use of the json gateway in favor of the underlying grpc endpoints.

avive commented 3 years ago

Okay - so the question is why do we support json/http at all, even for the non-streaming endpoints?

daonb commented 3 years ago

I've seen the CI code in the api branch and it does use the json streaming api but it's the wrong thing to do. It accesses the nodes through a curl instance running inside the namespace. We've all agreed that it's better for the test runner itself to run inside the namespace and use gRPC to access the nodes.

IMO, it's better to remove the json endpoints.

avive commented 3 years ago

I see no reason to keep them in the node level for 0.3 and I agree that we should better remove them so no new code relies on them.

lrettig commented 3 years ago

I don't have a strong opinion here. There's already support for the json gateway in the node, so I'm inclined to leave it there. Just have it off by default.

avive commented 3 years ago

I think we should get rid of the http-json gateway altogether and this will resolve this issue. See #151