I find the notion of re-adding the same subscriber with new filters is faulty. We now have two subscriptions from the same subscriber with different filters. The uniqueness of a subscription is not the subscriber to consider an overwrite, the combination of subscriber, event-type subscribed and filters applied.
Sandeep On Sun, Mar 23, 2014 at 6:04 PM, kishore g <[email protected]> wrote: > Actually Kanak has a valid point. we might need to re-add same subscriber > with to watch newly created resources. When we create the subscriber we > will not know all the resources upfront but we might be interested in > watching external view for resources that have fieldX=Y or some complex > logic like that. > > we need a way to update interested events dynamically. > > > > > On Sun, Mar 23, 2014 at 2:58 PM, Sandeep Nayak <[email protected]> wrote: > >> I will remove the connectionId, btw I reworked the EventSubscriber >> (its checked in) its exactly what you illustrated in your mail. It >> also gives the subscriber the benefit of registering once rather than >> multiple calls to register event subscriptions for each event. >> >> On Sun, Mar 23, 2014 at 2:52 PM, kishore g <[email protected]> wrote: >> > I don't see the need for connectionId. We should have a way to track >> > connections irrespective of having connection id or not. >> > >> > I like the idea of having subscriberId. >> > >> > Regarding filters, I think both points are valid. >> > - We don't want to make changes to EventService api, so pushing it to >> > subscriber interface allows us to expand subscriber functionality without >> > breaking compatibility. For instance, we don't have the ability to filter >> > currently and we can easily support the existing functionality and design >> > the filter later. >> > - EventSubscriber must be able to handle different types of events. I >> think >> > the subscriber interface must return List<Class<HelixEvent>> instead >> of >> > single one. >> > >> > I think we need some more time to design the filter api. I will be ok >> with >> > not providing the filter functionality right now, we can revisit it after >> > we get the api package ready. >> > >> > thanks, >> > Kishore G >> > >> > >> > >> > >> > >> > >> > >> > >> > On Sun, Mar 23, 2014 at 2:24 PM, Kanak Biscuitwala <[email protected] >> >wrote: >> > >> >> >> >> What I was saying is that instead of extending the AbstractSubscriber >> >> class, I think the same flexibility can be achieved by extending the >> Filter >> >> class. The problem with pushing filtering into the subscriber is that >> you >> >> have no flexibility on using the same EventSubscriber to handle two >> >> different types of events. >> >> >> >> Kishore, do you have any thoughts about this? >> >> >> >> Kanak >> >> ---------------------------------------- >> >> > Date: Sun, 23 Mar 2014 14:07:28 -0700 >> >> > Subject: Re: Helix API: Update for 03/23/2014 >> >> > From: [email protected] >> >> > To: [email protected] >> >> > >> >> > Hi Kanak, >> >> > >> >> > See comments inline >> >> > >> >> > On Sun, Mar 23, 2014 at 1:42 PM, Kanak Biscuitwala < >> [email protected]> >> >> wrote: >> >> >> >> >> >> Hi Sandeep, >> >> >> >> >> >> (1) Isn't the Filter class already generic enough to provide this >> level >> >> of pluggability? >> >> > [Sandeep] I did not change the Filter class, I simply made the >> >> > EventService take a Subscriber for subscription registration and >> >> > pushed the parameters to be interface methods on the Subscriber. That >> >> > way if we need subscribers to pass in more parameters during >> >> > subscription we enhance it through the Subscriber interface, we always >> >> > ask that they extend the AbstractSubscriber provided to shield >> >> > themselves from the Subscriber interface changes. >> >> > >> >> >> >> >> >> (2) Sure, I agree with this. >> >> >> >> >> >> (3) Looks fine. Do we need a ConnectionId and a SubscriberId? >> >> > >> >> > [Sandeep] The SubscriberId is tied to events subscribers so they can >> >> > unregister their subscriptions simply by passing in the id and not >> >> > having to hold on to the Filter and all the objects in the subscribe >> >> > call as it was before my changes. The connection id is used to track >> >> > connections for HelixConnection, we can do away with it if we think we >> >> > don't ConnectionId. >> >> > >> >> >> >> >> >> Kanak >> >> >> ---------------------------------------- >> >> >>> Date: Sun, 23 Mar 2014 13:15:36 -0700 >> >> >>> Subject: Helix API: Update for 03/23/2014 >> >> >>> From: [email protected] >> >> >>> To: [email protected] >> >> >>> >> >> >>> Here are the changes >> >> >>> >> >> >>> (1) I reworked the EventManager API to make it a bit more coarse >> >> >>> grained to allow easier extensions. >> >> >>> >> >> >>> (2) I have removed the id packages from model.id, statemachine.idinto >> >> >>> a singular package api.id since some of the other packages had one >> or >> >> >>> two classes and did not make sense to have too many packages just >> for >> >> >>> ids. >> >> >>> >> >> >>> (3) I added a api.client package with interfaces for >> >> >>> HelixConnectionFactory, HelixConnection, HelixSession as place >> holders >> >> >>> which will get fleshed out after discussions. I did not move the >> >> >>> HelixConnection from core> org.apache.helix as it has too many >> >> >>> dependencies. I think we need to rework it into API piece by piece >> >> >>> before we remove it. >> >> >>> >> >> >>> Please take a look. I will step away for a bit but will check back >> in >> >> soon. >> >> >>> >> >> >>> Thanks, >> >> >>> >> >> >>> Sandeep >> >> >> >> >> >> >> >>
