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

Feature Request - Listener Groups #338

Closed
ronag opened this issue Aug 27, 2016 · 11 comments
Closed

Feature Request - Listener Groups #338

ronag opened this issue Aug 27, 2016 · 11 comments

Comments

@ronag
Copy link

ronag commented Aug 27, 2016

Looking at the #211 feature it seems like any record can only have one listener. Which is a great first step!

However, I would like to take it further and be able to register a pattern + group name listener, i.e. I can have two different types of listeners working on the same record but providing different behaviours, e.g. composition, indexing, heartbeat etc... this is especially useful when working in a micro service based infrastructure.

Something like:

ds.record.listen('^foo/*', 'elasticsearch', (match, isSubscribed, response) => ...)
ds.record.listen('^foo/*', 'solr', (match, isSubscribed, response) => ...)
@yasserf
Copy link
Contributor

yasserf commented Aug 28, 2016

While I can see the appeal for this, this can currently be done by using smaller records rather than adding this sort of complexity on the server.

IE.

ds.record.get( 'foo/1-elasticsearch' )
ds.record.get( 'foo/1-solr' )

We can keep this issue open incase anyone would ever like to reference or comment on it, but I can't see this happening unless there's a very compelling usecase

@ronag
Copy link
Author

ronag commented Aug 28, 2016

I don't see how that solves the issue. If I have a table foo how would it index it to both elastic search and lucene in two different services?

e.g.

const todo1 = ds.record.get('todo/1')
const todo2 = ds.record.get('todo/2')
const todo3 = ds.record.get('todo/3')

// Now make all todos searchable through both es and lucene...

@yasserf
Copy link
Contributor

yasserf commented Aug 28, 2016

Our goal wasn't to use listening as a mechanism to subscribe to things people are listening to and manipulate the data, rather than provide data from other services into the application on demand.

Because of that, my previous comment assumed you wanted to pump data into records from different sources rather than copy the data from a record into multiple different systems.

I can see how it would work the other way round. The core team won't be able to look at this in the foreseeable future, but be interesting to see if anyone else uses listeners the same way.

@ronag
Copy link
Author

ronag commented Aug 28, 2016

Yes, the advantage of this is that we can index data into other services using only the deepstream API.

@yasserf
Copy link
Contributor

yasserf commented Aug 28, 2016

It's certainly an interesting benefit to use it that way. As I said, it does contradict with it's currently usecase, for example, we will now have a isProvided flag on record indicating that a listener is currently online and accepted a request.

I can see this being quite useful in the long run, but I'm afraid we won't be able to put this on our roadmap until we get alot more votes on it.

Since you don't really need to accept the subscription, given that you won't be providing data and it doesn't harm anyone else from being told ( not as badly as when providing at least ) it might actually be cheaper to get a hook into the current subscriptions on the system instead. That combined with a lock might work quite well. Just wondering if we could achieve this without polluting the listening API

@ronag
Copy link
Author

ronag commented Aug 28, 2016

@yasserf: Also, this would be useful also from the current use case, i.e. consider the case where I'm importing stock data from two different services?

e.g.

// import stock data from aidata
listen('stock/*', 'aidata', ...)

// import stock data from bloomberg
listen('stock/*', 'bloomberg', ...)

Hm, though accept/reject might actually cover this case. Assuming accept can be called asynchronously?

@ronag
Copy link
Author

ronag commented Aug 28, 2016

It seems that accept cannot be invoked asynchronously (at least not without warning), given https://github.com/deepstreamIO/deepstream.io-client-js/blob/feature/%23203-listen-functionality/src/utils/listener.js#L106. @yasserf: That is a bit of a major issue for us. We sometimes need to do a db lookup in order to determine whether a provider can accept the match or not. Alternatively, can a provider change its mind after accepting?

@yasserf
Copy link
Contributor

yasserf commented Aug 28, 2016

Good catch. This is purely just the deprecation mechanism, the server has a timeout that you can configure for how long a listener has before it is considered to not have answered:

https://github.com/deepstreamIO/deepstream.io/blob/master/conf/config.yml#L120

I'm looking to the client code now ( server is complete ), will see what other ways we can provide a deprecated message. Any ideas?

@ronag
Copy link
Author

ronag commented Aug 28, 2016

Any ideas?

Use a timeout before printing the deprecation message.

@yasserf
Copy link
Contributor

yasserf commented Aug 28, 2016

Use a timeout before printing the deprecation message.

Was hoping for something that didn't include having to store and remove timeouts for all the different listen callbacks, as well as having to decide what a default good amount of time is before removing it.

@ronag
Copy link
Author

ronag commented Aug 28, 2016

@yasserf: Alternatively, you could check for this._callback.length === 3.

@yasserf yasserf mentioned this issue Aug 29, 2016
@yasserf yasserf closed this as completed Aug 29, 2016
jaime-ez pushed a commit to jaime-ez/deepstream.io that referenced this issue Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants