Re: [Feedhenry-raincatcher] Ability to subscribe to more than one event at a time

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Feedhenry-raincatcher] Ability to subscribe to more than one event at a time

Wojciech Trocki
Hi Michael 

There are two side effects in Feedhenry sync that are important for your cases.

1) Line that was forwarded to us is responsible for triggering notification.
If you have specific notification then global notification will not be triggered.


IMHO both should be triggered as this is how most of the event implementation works.
This is trivial fix, but it may cause some backwards compatibility problems.

2) When you register notifications for specific datasets previous notifier will be removed
https://github.com/feedhenry/fh-sync-js/blob/6392b02813b09e252b1e7537249ce50dd8f5e5cb/src/sync-client.js#L118

IMHO this may relay on something like event emitter, but that will require a lots of changes in the library.

With that in mind - we cannot have more than 1 notification handler per dataset
RainCatcher team can provide wrapper for that method in RainCatcher that will allow us to notify more than one listener per dataset.

Sending to feedhenry-dev for other opinions, before executing any actions. 


On Fri, Oct 20, 2017 at 6:07 PM, Michael Hearne <[hidden email]> wrote:

There is a bug in sync where we cannot subscribe to events more than once at a time. The following fails:

workorderService.subscribeToDatasetUpdates(() => {
    ...
  });

https://github.com/feedhenry/fh-sync-js/blob/6392b02813b09e252b1e7537249ce50dd8f5e5cb/src/sync-client.js#L360-L361


--

MICHAEL HEARNE

MOBILE CONSULTANT

[hidden email]

@mickhearne  |  IM: #mhearne

_______________________________________________
Feedhenry-raincatcher mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-raincatcher




--

WOJCIECH TROCKI

Red Hat Mobile

IM: wtrocki


_______________________________________________
feedhenry-dev mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Feedhenry-raincatcher] Ability to subscribe to more than one event at a time

Wei Li
+1 on using the event emitter. It should be the default implementation for the js sync client library.

I also think this should be fixed inside the sync framework itself. 

For the js client, one options is that we could init an event emitter for each dataset. Then the event emitter can be used to manage the events for the bound dataset. I think no changes to the sync API required to support this.

On Mon, Oct 23, 2017 at 10:02 AM, Wojciech Trocki <[hidden email]> wrote:
Hi Michael 

There are two side effects in Feedhenry sync that are important for your cases.

1) Line that was forwarded to us is responsible for triggering notification.
If you have specific notification then global notification will not be triggered.


IMHO both should be triggered as this is how most of the event implementation works.
This is trivial fix, but it may cause some backwards compatibility problems.

2) When you register notifications for specific datasets previous notifier will be removed
https://github.com/feedhenry/fh-sync-js/blob/6392b02813b09e252b1e7537249ce50dd8f5e5cb/src/sync-client.js#L118

IMHO this may relay on something like event emitter, but that will require a lots of changes in the library.

With that in mind - we cannot have more than 1 notification handler per dataset
RainCatcher team can provide wrapper for that method in RainCatcher that will allow us to notify more than one listener per dataset.

Sending to feedhenry-dev for other opinions, before executing any actions. 


On Fri, Oct 20, 2017 at 6:07 PM, Michael Hearne <[hidden email]> wrote:

There is a bug in sync where we cannot subscribe to events more than once at a time. The following fails:

workorderService.subscribeToDatasetUpdates(() => {
    ...
  });

https://github.com/feedhenry/fh-sync-js/blob/6392b02813b09e252b1e7537249ce50dd8f5e5cb/src/sync-client.js#L360-L361


--

MICHAEL HEARNE

MOBILE CONSULTANT

[hidden email]

@mickhearne  |  IM: #mhearne

_______________________________________________
Feedhenry-raincatcher mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-raincatcher




--

WOJCIECH TROCKI

Red Hat Mobile

IM: wtrocki


_______________________________________________
feedhenry-dev mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-dev




--

WEI LI

SENIOR SOFTWARE ENGINEER

Red Hat Mobile

[hidden email]    M: <a href="tel:+353862393272" style="color:rgb(0,136,206);font-size:11px;margin:0px" target="_blank">+353862393272    


_______________________________________________
feedhenry-dev mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Feedhenry-raincatcher] Ability to subscribe to more than one event at a time

Wojciech Trocki
Great to have agreement!
We can do that change directly in sync without compromising it's api. 
Created ticket for that in RainCatcher project.

On Mon, Oct 23, 2017 at 12:10 PM, Wei Li <[hidden email]> wrote:
+1 on using the event emitter. It should be the default implementation for the js sync client library.

I also think this should be fixed inside the sync framework itself. 

For the js client, one options is that we could init an event emitter for each dataset. Then the event emitter can be used to manage the events for the bound dataset. I think no changes to the sync API required to support this.

On Mon, Oct 23, 2017 at 10:02 AM, Wojciech Trocki <[hidden email]> wrote:
Hi Michael 

There are two side effects in Feedhenry sync that are important for your cases.

1) Line that was forwarded to us is responsible for triggering notification.
If you have specific notification then global notification will not be triggered.


IMHO both should be triggered as this is how most of the event implementation works.
This is trivial fix, but it may cause some backwards compatibility problems.

2) When you register notifications for specific datasets previous notifier will be removed
https://github.com/feedhenry/fh-sync-js/blob/6392b02813b09e252b1e7537249ce50dd8f5e5cb/src/sync-client.js#L118

IMHO this may relay on something like event emitter, but that will require a lots of changes in the library.

With that in mind - we cannot have more than 1 notification handler per dataset
RainCatcher team can provide wrapper for that method in RainCatcher that will allow us to notify more than one listener per dataset.

Sending to feedhenry-dev for other opinions, before executing any actions. 


On Fri, Oct 20, 2017 at 6:07 PM, Michael Hearne <[hidden email]> wrote:

There is a bug in sync where we cannot subscribe to events more than once at a time. The following fails:

workorderService.subscribeToDatasetUpdates(() => {
    ...
  });

https://github.com/feedhenry/fh-sync-js/blob/6392b02813b09e252b1e7537249ce50dd8f5e5cb/src/sync-client.js#L360-L361


--

MICHAEL HEARNE

MOBILE CONSULTANT

[hidden email]

@mickhearne  |  IM: #mhearne

_______________________________________________
Feedhenry-raincatcher mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-raincatcher




--

WOJCIECH TROCKI

Red Hat Mobile

IM: wtrocki


_______________________________________________
feedhenry-dev mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-dev




--

WEI LI

SENIOR SOFTWARE ENGINEER

Red Hat Mobile

[hidden email]    M: <a href="tel:+353862393272" style="color:rgb(0,136,206);font-size:11px;margin:0px" target="_blank">+353862393272    




--

WOJCIECH TROCKI

Red Hat Mobile

IM: wtrocki


_______________________________________________
feedhenry-dev mailing list
[hidden email]
https://www.redhat.com/mailman/listinfo/feedhenry-dev