vert-x3 / vertx-tcp-eventbus-bridge

Apache License 2.0
49 stars 44 forks source link

Implement JSONRPC as wire protocol #63

Closed nbrendah closed 3 years ago

nbrendah commented 3 years ago

Motivation:

Explain here the context, and why you're making that change, what is the problem you're trying to solve. I have refactored the documentation to support JSON-RPC using https://www.jsonrpc.org/specification#id1 cc @pmlopes

Conformance:

Your commits should be signed and you should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines

nbrendah commented 3 years ago

Since i am new here, i honestly don't know what to include under the Motivation and Conformance sections

pmlopes commented 3 years ago

Hi @nbrendah let me expand a bit more about the project. Vertx event bus allows developers to write message driven applications that scale. However not all applications run on the java virtual machine, so having a bridge in a common stack like TCP IP permits us to reach any programming language or framework.

Given that we don't want to break any existing application, adding a new protocol means we need to support both the previous one and give a warning that users should upgrade and the new one.

In this case we want to support json RPC 2.0 in this bridge.

While the protocol itself is not complicated, there are some design challenges.

For example, json RPC works in a request, response way, however vertx event bus can also do push notifications.

This can be done in json RPC too if a node acts both as a server and a client. This may sound awkward but in fact is discussed in the json RPC spec and forum.

Second although the name of the protocol is json RPC there are clients capable of using other encodings, what matters is the message format.

In the end we would like to have a bridge capable of connecting to https://www.npmjs.com/package/vscode-jsonrpc

jnsereko commented 3 years ago

@pmlopes Thank you for the great explanation. It's a pretty nice tool, you got. I have read all the links you shared on discord and watched some YouTube about this.

Well, as far as I know, this is something big, and honestly for the start is looks almost impossible for a starter like me.

How about we break it furthermore? Or, a better solution would be to open up a public thread on the discussion platform to get some steps and more info of where the fix is needed and how it's needed.

I might not be pain working with. I think after some few hows become pretty clear with me, questions are gonna become less and GitHub commits are going to do the talking.

Thanks @pmlopes for the good work.

pmlopes commented 3 years ago

Hi @jnsereko don't get down if the project looks complex and big. In fact you can find a branch where some work has been done already.

https://github.com/vert-x3/vertx-tcp-eventbus-bridge/tree/experimental/jsonrpc

The challenge now (and engineering task) is to verify if we can use jsonrpc. What should be the expected message exchange between the bridge and it's consumer or producers.

If we manage to define this model messages then it's just coding. Transform the expectations to tests, and from there implement the code to ensure tests pass.

Sure it's some work, a lot if you're just beginning, but I'll try to assist anyone and am confident we can make it work!

nbrendah commented 3 years ago

Hi @pmlopes Been thinking de trop on how to implement this. I was really frightened at the start but this is not as complex as i though before, thanks to your previous comments. The project supports TCP and interacts with any system that can create TCP sockets. If we need to add on support for JSON-RPC:

  1. How are we going to determine which protocol to use?
  2. I think the very first step is to add JSON-RPC-2 base dependence. Should i go ahead and send a PR for that?

Thank you so much

nbrendah commented 3 years ago

closing this because it was off topic.