search

onflow / flow-go-sdk

Tools for building Go applications on Flow :ocean:
https://onflow.org
Apache License 2.0
212 stars 87 forks source link

Proposal: Deprecate HTTP client support #743

Open peterargue opened 1 week ago

peterargue commented 1 week ago

Currently the SDK support both gRPC and REST base access clients. Both clients seem to provide the same API and are functionally equivalent. However, the gRPC API has more features and is more robust out of the box for go clients.

Maintaining 2 versions of the client with identical functionality adds maintenance overhead and will likely cause odd behavioral differences since the onflow/flow-go versions of the APIs behave slightly differently.

I propose we align on using the gRPC client, and deprecate and remove the HTTP client.

bluesign commented 1 week ago

I would consider removing grpc tbh, this creates incentives to keep rest api in sync with grpc ones.

AN and Client communication is better handled with REST for sure. ( load balancing, no need for envoy, tls etc )

nvdtf commented 1 week ago

I remember some clients preferred to have encrypted communications with the AN that was not possible with gRPC so they used REST. Not sure if this line of reasoning still applies.

jribbink commented 1 week ago

Yeah, my comment was going to be exactly to the same effect of @bluesign. I know FCL has had issues in the past where GRPC and HTTP did not maintain parity.

Now that GRPC-Web is deprecated in FCL, parity is especially important for JavaScript clients. As HTTP tends to be a more universal transport layer, this could also apply to any other integrations where adding low-overhead, natively-supported HTTP requests is the preferrable (or only possible) option. If the Go SDK deprecates HTTP, I would hope that flow-go still maintains reasonable parity for other HTTP clients.

peterargue commented 1 week ago

I would consider removing grpc tbh, this creates incentives to keep rest api in sync with grpc ones.

Regardless of what we decide here, the goal is to maintain parity in flow-go between REST and grpc. There are going to be differences due to the limitations specific to REST/websockets, but we intend to support both.

AN and Client communication is better handled with REST for sure. ( load balancing, no need for envoy, tls etc )

I don't agree. Overall I think that grpc is a better technology for managing long lived backend application connections, which are pretty common for the go-sdk's usecases. It also uses much less bandwidth than REST.

Load balancing is less of an issue now that the public infrastructure handles this.

I remember some clients preferred to have encrypted communications with the AN that was not possible with gRPC so they used REST. Not sure if this line of reasoning still applies.

This is a good point. Nodes do support TLS over grpc, but we haven't pushed to have that enabled for the public infrastructure. We certainly can.

@nvdtf do you know if these users wanted CA signed certificates, or were they OK with the node signed certs?

peterargue commented 1 week ago

I'm not attached to keeping grpc over rest (though I am biased). I'm just looking to reduce the maintenatnce overhead of maintaining both. For context, we're kicking off a project to bring the go-sdk up to speed with the latest API updates (epic). Implementing these for both the grpc and rest seems like a waste of cycles, especially once we get to the streaming endpoints.

nvdtf commented 6 days ago

I don't remember the exact requirements, but there was a time we only supported gRPC and clients pushed for HTTP support mentioning reasons like security, better infrastructure support for firewalls, better monitoring of traffic, ...

bluesign commented 6 days ago

I am a bit biased too maybe, not that I trust you but tbh I think REST will become less and less capable.