[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user toyboxman commented on the issue: https://github.com/apache/incubator-griffin/pull/444 It's long process of discussion and review, but we get a consensus finally and close this change. thanks guys @chemikadze @gavlyukovskiy @guoyuepeng ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user gavlyukovskiy commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Yes, I showed this in my example - take all hooks from the context, but use only those that configured in property. ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 >> > Can we provide several individually implements in configuration like GriffinHook1, GriffionHook2, and autowired all individual hooks instances into List? >> >> You can

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user gavlyukovskiy commented on the issue: https://github.com/apache/incubator-griffin/pull/444 > Can we provide several individually implements in configuration like GriffinHook1, GriffionHook2, and autowired all individual hooks instances into List? You can define all

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 I am ok for registration through declaration in properties. my problem is can we write triggered on called test case for in EventServiceTests. ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Based on previous design discuss and code review. LGTM now. Any comments from you? @chemikadze ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user gavlyukovskiy commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Left a comment regarding injection. Tests didn't catch it because you explicitly specified all hooks that are registered in the context, I would add one more and leave it unused

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user toyboxman commented on the issue: https://github.com/apache/incubator-griffin/pull/444 summary of comments 1.considering general usage, I do not use 'GriffinJobEventManager' but actually name it 'GriffinEventManager' 2.I keep empty implementation for

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 @chemikadze right, we cannot tie events to jobs related. @toyboxman ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user chemikadze commented on the issue: https://github.com/apache/incubator-griffin/pull/444 @toyboxman leaving minor comments aside, looks really nice! ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user chemikadze commented on the issue: https://github.com/apache/incubator-griffin/pull/444 Regardig naming -- do you think we should really tie it to Jobs? Handling Measure events could be useful as well. ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user toyboxman commented on the issue: https://github.com/apache/incubator-griffin/pull/444 let me change name to GriffinJobEventManage ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user guoyuepeng commented on the issue: https://github.com/apache/incubator-griffin/pull/444 cool! how about rename GriffinHookChain as GriffinJobEventManager so similar? ---

[GitHub] incubator-griffin issue #444: Define griffin plain-vanilla hook.

Github user toyboxman commented on the issue: https://github.com/apache/incubator-griffin/pull/444 @guoyuepeng @chemikadze Thanks for your comments on first version of hook implementation. I have completed new implementation, here I simplify hook load, extend event