Hi, Le lun. 29 oct. 2018 à 17:42, Stefan Egli <[email protected]> a écrit :
> Right, Sling Jobs can help in defining the deprecation, which is exactly > what I've use it for now. (btw: I went even further in restricting the > API, as I think retryJobById and getJobById are potentially expensive as > well, thus lean towards deprecating those too +1 with deprecating retryJobById and getJobById. Those signatures hint that the implementation will require random access to jobs which I expect to be inefficiently supported on top of a messaging infrastructure. > - not sure about > getScheduledJobs...). > I think it makes sense to deprecate it for the reasons you shared in your initial message. > > Re new vs deprecation: I'd say if we would see need for changing the > actual API, ie the signatures, other than plain deprecating it, that > would be an argument for a new API. So I see that as the key question. > As if we just deprecate, there are of course several advantages: those > that don't violate anything can keep their code unchanged - the > implementation remains the same for now - no need to implement > additional, probably slightly different API... - no flags needed.. > I am also in favour or deprecating without replacement, without starting a new API. Cheers, Timothee > > Cheers, > Stefan > > On 29.10.18 17:15, Timothee Maret wrote: > > Hi, > > > > This thread reminds me that Ian built Sling Jobs module [0]. IIUC from > > reading the project readme, Sling Jobs design focused on supporting the > > APIs from the Sling Events module that can be implemented efficiently on > a > > messaging system. > > > > I am thinking that the Sling Jobs API could help defining the Sling > Events > > API to be deprecated or could serve as a new API (inline with Robert > > suggestion). > > > > Cheers, > > > > Timothee > > > > > > [0] > > > https://github.com/apache/sling-org-apache-sling-jobs#apache-sling-jobs-support > > > > Le lun. 29 oct. 2018 à 16:53, Robert Munteanu <[email protected]> a > écrit : > > > >> Hi Stefan, > >> > >> On Mon, 2018-10-29 at 14:47 +0100, Stefan Egli wrote: > >>> Hi, > >>> > >>> I'd like to reactivate SLING-5884 and a related discussion [1] we > >>> had > >>> about deprecating the query part of the Sling Event API. > >>> > >>> TL;DR: Open question is exactly which of the JobManager methods do > >>> we > >>> want to deprecate. > >> > >> > >> How about depreacting the JobManager completely and creating new API > >> which does exactly what we want? > >> > >> We can switch the JobManagerImpl to being a component registered only > >> if a certain configuration property is set. We can set the default to > >> 'false' and log a WARN when we register that component to make it clear > >> that it's not going to be supported. Eventually we can stop registering > >> the implementation outright and make the inconvenient methods throw an > >> UnsupportedOperationException. But I would prefer to create a new API > >> for new 'realities'. > >> > >> I know the transition will be more inconvenient, but a new API will > >> also force clients to make conscious choices about what they want to > >> support and a clear transition path. > >> > >> While not giving it a lot of thought to the names and the API methods > >> we could support, here's a strawman API we can bash: > >> > >> ---------------- > >> > >> // do we need both submit and create? > >> interface JobManager2 { > >> SubmittedJob submitJob(...) > >> JobBuilder createJob(...) > >> } > >> > >> // do we need both stop and remove? > >> interface SubmittedJob { > >> void retryJob() > >> void removeJob() > >> void stopJob() > >> } > >> > >> interface JobStatisticsManager { > >> getQueue(...); > >> getQueues(); > >> getStatistics(); > >> // etc, etc > >> } > >> > >> ---------------- > >> > >> The separation of concerns makes it quite clear what is possible and > >> what not, and additionally the client is required to hold on to the > >> SubmittedJob if they want to do anything more with it. > >> > >> We also might want to use exceptions instead of returning null in the > >> API, might make things clearer. > >> > >> Thoughts? > >> > >> Robert > >> > >> > >>> > >>> > >>> The idea is to deprecate (and sometime in the future - timing tbd - > >>> remove) those methods of the sling event api which we either > >>> consider > >>> expensive to support (eg maintaining large indexes for queries) or > >>> making assumptions/limitations on the underlying implementations and > >>> thus prohibit usable alternative implementations such as messaging- > >>> based > >>> ones. > >>> > >>> > >>> Now to the discussion as to what exactly we want to deprecate and how > >>> we > >>> see alternatives (or not) for each (all methods of JobManager [2]): > >>> > >>> (A) addJob, createJob: Those must stay > >>> > >>> (B) getQueue(s), get(Topic)Statistics: Those can probably stay > >>> > >>> (C) getJob, findJobs (and QueueType): those are the main query > >>> methods > >>> that should be deprecated. We'd not provide an alternative, but > >>> could > >>> suggest those who want to maintain job information could do that > >>> themselves in a shared place. > >>> > >>> (D) getJobById, retryJobById: both assume some kind of job storage > >>> that > >>> can be accesses in an arbitrary way. Thus I'd argue both could be > >>> deprecated. Alternative suggestion similar to (C). Edge-case. > >>> > >>> (E) removeJobById: can imv stay. the api makes no hard requirement > >>> to > >>> check if the job ever existed, however it requires that the job be > >>> removed when returning true, so requires (expensive) synchronicity. > >>> Doable. Edge-case. > >>> > >>> (F) stopJobById: can imv stay. > >>> > >>> (F) getScheduledJobs: while this requires some form of job storage > >>> it > >>> only affects a small amount of data. But if we'd want to make the > >>> job > >>> api messaging-ready it would also have to be deprecated. Edge-case. > >>> > >>> > >>> Cheers, > >>> Stefan > >>> -- > >>> [0] - https://issues.apache.org/jira/browse/SLING-5884 > >>> [1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j > >>> [2] - > >>> > >> > https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java > >> > >> > >> > > >
