-
Notifications
You must be signed in to change notification settings - Fork 283
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
refactor(cmd-api-server): add socket.io connection to each ledger-connector #1602
Comments
@takeutak Could you please expand on this part? |
@petermetz Thanks for your advice! Let me clarify one thing, is the following figure right for calling your mentioned codes?
According to the figure, BLP sends requests to API clients on each ledger connector via socket.io connection (*1), and API clients sends requests to ledger connector via socket.io connection (*2), is it right? I think your implementation is good, but it makes more useful to prepare the following ways:
To be clear, this issue is not intended to remove the features you already developed. It is only intended to enhance the features. |
@petermetz Please review this implementation idea and share your comments (especially on preferred location of new components - SocketIOApiClient and Verifier) The general idea is to divide the current Verifier from cmd-socketio into two components that will be more compatible with api-server architecture. SocketIOApiClient
Verifier (ledger monitor proxy)
While implementing this I'll try to be api-server native (keeping #1502 in mind). I'll try to use this common code implementation in cmd-socketio to prevent code duplication, but it will be lower priority for me (if that's OK). |
@outSH Thank you for the details. How will the |
@petermetz I don't have a definite plan for that yet, I'll experiment and see what fits the best. I was thinking about replacing Do you have any other comments regarding this feature? |
More details would be very welcome as they come along. The reason I'm highly interested in the details of this specifically because the "common abstraction" seems to be the foundational premise for everything else that's in the issue.
Any chance that the filename conventions in place could be kept? (lower-hypen-case)
Calling it SocketIOApiClient would imply that there hasn't been SocketIO support added already a long time ago to the other one. Is there something else that refers to a real distinction that we could put in the name of it? Is the |
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
I've finished an initial implementation, published as draft PR here -> #1660. Please review and share your comments. There are some more things to do, but the main logic is already there.
In the end
Yes, of course, it was a copy-paste typo.
Yes, I'm not satisfied with the name either, ( but unfortunately couldn't think of anything more suitable, I'm very open to suggestions :) It's meant to work with current, non-openapi based socketio interfaces, maybe we could add
I made a change and Verifier has a one-to-one relation to the ApiClient now. |
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
@outSH Got it, thank you for the adjustments so far. I'll head over to the draft PR and we can have a much more targeted discussion over there.
Based on my hope that we'll end up consolidating I would go with
Awesome, thank you for updating it like that! |
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in #1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: #1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
… api-server and api-client Full description of changes and planning log available in hyperledger-cacti#1602. In general, this commit adds two new components - one for communicating with plain socketio validators using socketio interface, and verifier that implements similar features as socketio-server verifier. Closes: hyperledger-cacti#1602 Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com> Signed-off-by: aldousalvarez <aldousss.alvarez@gmail.com>
Is your feature request related to a problem? Please describe.
On the current codes, cmd-api-server does not have socket.io connection to each ledger-connector. This communication channel is needed for developing the APIs of sendSyncRequest, sendAsyncRequest, and startMonitor as these are developed in cmd-socket-server via socket.io connection to each ledger.
This feature was already developed in cmd-socket-server, but in case of merging those codes as #1502, we need to develop it.
Describe the solution you'd like
Add the socket.io connection to cmd-api-server
EPIC
#1605
The text was updated successfully, but these errors were encountered: