[Relayminer] add forward capabilities to send request to suppliers#1147
[Relayminer] add forward capabilities to send request to suppliers#1147
Conversation
|
The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. You may need to run GCP workloads (requires changing the namespace to 1147) |
|
Here is the new PR @Olshansk ! |
69874db to
ed0530c
Compare
|
@eddyzags I've added the |
Olshansk
left a comment
There was a problem hiding this comment.
@eddyzags Thanks for the change!
- A few minor nits/comments/edits
- Please merge with main
- In case you missed it, do check out
make send_relay_path_WEBSOCKET: it's websockets leveraging the PATH gateway - Can you provide some more detail/rationale on the:
- Token
- Use case
- Maybe an E2E mermaid diagram of how this works
I'm still struggling to understand what use case this serves that was possible before, but am pretty sure it's just an oversight on my part.
| return nil | ||
| } | ||
|
|
||
| // forwardAsyncConnection instantiates two websocket connections that: |
There was a problem hiding this comment.
Can you #PUC why we need two websockeet connections and one is not enough?
There was a problem hiding this comment.
It is written in the comment (103 & 104).
I need two connections to proxy the websocket traffic from the client to the backend websocket service. One connection (clientConn) between your proxy and the client, and one connection (serviceConn) between your proxy and the backend service.
|
Delegating to @eddyzags to answer this question. |
Yes. As stated in this issue #447 , the objective is to:
I don't have any opinions on whether this should be in the next iteration or a non-blocking release. But, the From an OSI standpoint, the I think we should give both capabilities to the Relayminer's users. |
e9b90a9 to
647cf9d
Compare
50a0518 to
16b0535
Compare
|
Friendly reminder @bryanchriswhite @Olshansk 👍🏾 |
|
@eddyzags Do you mind resolving the conflicts with main so we can re-review and get this in? |
|
@Olshansk, there were quite a few changes to this part in the main branch. I'll have to understand it first to resolve the conflicts while keeping the code working. I am working on it. I try to push the changes as soon as I can. |
|
@Olshansk I got mixed up with the rebase. I was better and faster for me to start from a clean slate. See follow-up here: #1681 @bryanchriswhite I am still waiting for some answers on some discussions in this PR. You can either answer here or in the new PR, I'll backport the modifications. Sorry for the inconvenience. In the meantime, I am closing this PR. Follow-up available here: #1681 |
Summary
This PR aims to add capabilities to forward requests to HTTP and a WebSocket server in the relayminer.
It sets up a configurable authenticated endpoint (/services/{service_id}/forward) whose responsibility would be to forward raw requests from the Relay Server to the service ID (more precisely, service node or data node).
Issue
Type of change
Select one or more from the following:
consensus-breakinglabel if so. See [Infra] Automatically add theconsensus-breakinglabel #791 for detailsSanity Checklist
assignees,reviewers,labels,project,iterationandmilestonemake docusaurus_startmake go_develop_and_testandmake test_e2edevnet-test-e2elabel to run E2E tests in CI