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

API consistency for auto:false & ability to observe/consume on same port #34

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pdspicer
Copy link

@pdspicer pdspicer commented Jun 6, 2016

If I set up seneca-mesh as follows with a static port and auto:false

seneca({transport: {host: ip, port: 12345}})
    .use('mesh', {auto: false, isbase: true, host: ip, bases: [ip:12345], port: 23456})
    .ready(function() {
        this.listen({pins: ['foo:1', 'bar:2'], model: 'consume'})
    })

I lose the ability to specify a model per pin. I would like to be able to specify my pins as follows:

this.listen({pins: [{pin: 'foo:1', model: 'consume'}, {pin: 'bar:2', model: 'observe'}]})

kind of like you can if you use auto: true.
If I am running on multiple machines inside a network with strict firewall restrictions, being able to specify explicit ports is a valuable feature. But the current implementation seems to allow this at the expense of the flexibility to specify different models on a single port.

Since the models are only actually utilized by the balance-client, it seems as though allowing both observe and consume on a single port should be possible. This improves the consistency of the API for listen between auto:true and auto:false, and increases the flexibility of the auto:false listen() calls. You can also do interesting things like change the default model to observe, if you set the options as follows:

this.listen({model: 'observe', pins: [{pin: 'foo:1', model: 'consume'}, 'bar:2']})

then any pins defined as strings will default to observe unless otherwise specified

Paul Spicer added 2 commits June 6, 2016 04:20
@pdspicer pdspicer changed the title PR-34: API consistency for auto:false & ability to observe/consume on same port API consistency for auto:false & ability to observe/consume on same port Jun 7, 2016
@rjrodger
Copy link
Collaborator

@pdspicer this is a legitimate feature. I'm just about to release 1.0.0 and will review again once that is out.

@pdspicer
Copy link
Author

Will keep an eye out for review, thanks

@mcdonnelldean
Copy link
Contributor

@pdspicer I think that is the last of the big changes in. I did try to merge but I'm short on time so didn't manage to get it resolved. If you can resolve the conflict we can look into this more. As Richard said, looks like a valid use case and one we should look at soon after release.

@pdspicer
Copy link
Author

pdspicer commented Sep 5, 2016

Managed to get it merged in, changes should only affect behavior of the add_client() function

@mcdonnelldean
Copy link
Contributor

Awesome! @rjrodger Have a quick eye over. LGTM. I'll merge in tomorrow and publish if all ok. This should make mapping out meshes nicer.

@rjrodger
Copy link
Collaborator

rjrodger commented Mar 6, 2017

@pdspicer Just reviewing this again - you are able to use multiple listens, like so:

seneca.use('mesh', {
  listen: [
    {pin: 'get:content'}, // model:consume; the default
    {pin: 'clear:cache', model:'observe'}
  ]
})

Does this achieve what you need?

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