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

Http Upstream client #197

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cesar-startale
Copy link

This pull request introduces the HTTP Upstream client. The main goal is to be backward compatible and introduce an HTTP upstream client.

If the config has any HTTP/s server to connect to, the HttpClient struct will handle them. Any WebSocket will use the existing upstream client code.

If one or more HTTP clients are configured, they route all requests, and Websocket clients will be used for subscriptions.

If no HTTP clients are configured, the fallback behavior is used, and all requests and subscriptions are routed through the WebSocket upstream client.

If no WebSocket upstream clients are configured, then subscriptions are not enabled, only method requests.

This pull request introduces the HTTP Upstream client. The main goal is to be
backward compatible and introduce an HTTP upstream client.

If the config has any HTTP/s server to connect to, the HttpClient struct will
handle them. Any WebSocket will use the existing upstream client code.

If one or more HTTP clients are configured, they route all requests, and
Websocket clients will be used for subscriptions.

If no HTTP clients are configured, the fallback behavior is used, and all
requests and subscriptions are routed through the WebSocket upstream client.

If no WebSocket upstream clients are configured, then subscriptions are not
enabled, only method requests.
@ermalkaleci
Copy link
Collaborator

If you want to connect to http upstream then consider using a reverse proxy. Subway was built exactly to deal with websocket as there's no tool for it. Subway depends on websocket and that's the only thing it should deal with. This cannot be a tool for every problem.

@cesar-startale
Copy link
Author

Hello @xlc, We plan to add an HTTP upstream client. Would contributing upstream be a good idea?

The main goal is to route method calls to HTTP upstream clients and use the WebSocket upstream clients for subscriptions.

The plan for this PR to be ready is as follows:

  • Finish up HTTP upstream client: I am currently working on a Keep-Alive or HTTP/2 configuration that can reuse open connections.
  • This should be 100% backwards compatible configuration-wise. Add more tests around this.
  • Explore how to contribute back. If this contribution does not align with Subway's roadmap, I am exploring packaging this upstream client as self-contained in a crate. People can choose to use the built-in client or our version at compile time. Minor changes are needed for this to happen. I don't wish to introduce traits but have the subway::extensions::client::Client be a type, pub type Client = ConcretClientImpl.

Introduce the meta Client that will instantiate the original Ws client or the
new Http upstream client
@xlc
Copy link
Member

xlc commented Jul 21, 2024

I see some use cases this could be helpful. e.g. enable WS support for endpoint that only supports HTTP JSON RPC and the implementation shouldn't be too complicated (i.e. not much overhead to maintain it). So it should be a nice additional to have.

@ermalkaleci
Copy link
Collaborator

The only way to achieve the so-called 'subscription' will be to repeatedly send requests to the HTTP server. I don't think it is efficient.

@xlc
Copy link
Member

xlc commented Jul 24, 2024

Yeah I don't think there will be subscription support with HTTP upstream. It should only be enabled with WS upstream.

@ermalkaleci
Copy link
Collaborator

ermalkaleci commented Jul 24, 2024

The main goal is to route method calls to HTTP upstream clients and use the WebSocket upstream clients for subscriptions.

For HTTP route calls it should be fine, but still there's the need to constantly call the HTTP repeatedly to get latest/finalized block but that is fine.
Edit: I still don't like this feature but we can give it a try

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants