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

No namespace and a strange "type" for Requester and Responder instances #236

Open
joelguittet opened this issue Dec 14, 2020 · 7 comments
Open

Comments

@joelguittet
Copy link

joelguittet commented Dec 14, 2020

Hello @dashersw

Following development of the c-cote library, I notice that the Requester and Responder do not use namespace and topic at all, instead a "type" field is required to have a kind of topic.

Is it something that is really wanted ? or working on the API still needed ? What append if "type" is an applicative information the user want to send ?

Actually the API of the publisher looks like:

randomPublisher.publish('topic', json);

I imagine Requester can be :

randomRequest.send('topic', json, function(res) {
    //handle res
});

With usage of the topic and namespace of course.

I have created a small doc on messages format at https://github.com/joelguittet/c-cote/wiki/Format-of-Cote-messages. In my dreams the format of messages between Requester and Responder looks like between Publisher and Subscriber:

+-----------+-------------+-------------+-------+-------------+----------------+
| Fulltopic | AMP field 1 | AMP field 2 | ..... | AMP field N | `pid:id` field |
+-----------+-------------+-------------+-------+-------------+----------------+

With fulltopic : message::<namespace>::<topic>

Note I suggest on this schema to permit sending any type of AMP fields (blob - useful for sending audio, json, string and bigint), and several fields of course, because it's permitted by axon. The "pid:id" is added by axon itself.

This can be the beginning of a discussion on the subject if you want. Your namespace idea is really not bad and will be a nice to have on Req/Rep in my opinion.

Joel

EDIT: just seen this issue too: #165. Not sure about this concept of "subset" instead of namespace.

@dashersw
Copy link
Owner

For some reason I hadn't seen this issue! Sorry.

Namespaces are mainly intended for the Sockend component, and directly map to socket.io namespaces. However, if you look at the Component.js line 29 you will notice that all components make use of namespaces in discovery and connection.

The main reason for why the requester / responder syntax is not like randomRequest.send('topic', json, function(res) { is because the goal was to keep the request object very simple — build a single object however you like, so you don't have to use multiple parameters. On the other hand, sending blobs is something we need to figure out. That was on our list.

Also, yes, I love this discussion and would love to think of a cote v2.

You can also check out kotelett, my attempt to simplify this, plus make connections dynamic, with even less configuration, based on what messages a service could respond to. I intended kotelett to be the next generation of cote.

@joelguittet
Copy link
Author

Ok for the namespace, you're right, it's used on discovery.

For the topic subject, that's purely some API/syntax question because I was surprised of the 'type' field. What append if the 'type' is not filled ? Not tested on my side but probably it's not working ?

Joel

@dashersw
Copy link
Owner

If there's no type in a request object, the responder throws it away, as it's not a legitimate request.

@dashersw
Copy link
Owner

Please check out kotelett. Your suggestion could be easily implemented there, and it's in fact much simpler to use than cote.

@joelguittet
Copy link
Author

Well it's not a problem for me, just surprising. On publisher you have:

randomPublisher.publish('update3', { val: 'should not be getting this' });

You have not:

randomPublisher.publish({ type: 'update3', val: 'should not be getting this' });

That's why I said it's only an API/syntax convenience/uniformity across cote library but for my own usage it does not matter.

@otothea
Copy link
Contributor

otothea commented Jan 27, 2021

Well it's not a problem for me, just surprising. On publisher you have:

randomPublisher.publish('update3', { val: 'should not be getting this' });

@joelguittet This threw me off when I first started using cote as well. First time I tried using publisher i couldn't get it to work until i realized it takes type as a function argument instead of a key on the payload object.

@joelguittet
Copy link
Author

@otothea yeah exactly, publisher is one way and requester is another way. Troubling.

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

No branches or pull requests

3 participants