Github user chemikadze commented on a diff in the pull request:
https://github.com/apache/incubator-griffin/pull/444#discussion_r229565134
--- Diff:
service/src/main/java/org/apache/griffin/core/job/JobServiceImpl.java ---
@@ -168,7 +175,14 @@ public AbstractJob addJob(AbstractJob job) throws
Exception {
Long measureId = job.getMeasureId();
GriffinMeasure measure = getMeasureIfValid(measureId);
JobOperator op = getJobOperator(measure.getProcessType());
- return op.add(job, measure);
+ AbstractJob jobSaved = op.add(job, measure);
+ JobEvent jobEvent = new JobEvent(jobSaved);
+ eventListeners.forEach(hook -> {
--- End diff --
Feels like passing hooks as list requires all injection points to be aware
of execution semantics of hooks. Hiding list of hooks behind some HookChain
interface and having `@Bean HookChain` instead of `@Bean List<>` should solve
this problem. HookChain would do dispatching of event to all hooks in a list,
while all hook uses would be simplified to just `hookChain.onEvent(event)`.
Also I think that in discussions there was an agreement to have blocking
execution semantics by default, rather than asynchronous dispatching. To
simplify asynchronous hook development, abstract class AsynchronousHook can be
provided, which would dispatch calls of abstract onEventAsync method to
internal (?) thread pool.
---
