[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 just

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

Github user gavlyukovskiy commented on a diff in the pull request: https://github.com/apache/incubator-griffin/pull/444#discussion_r231981734 --- Diff: service/src/main/java/org/apache/griffin/core/event/GriffinEventListeners.java --- @@ -0,0 +1,50 @@ +package org.apache.griff

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

Github user guoyuepeng commented on a diff in the pull request: https://github.com/apache/incubator-griffin/pull/444#discussion_r231798479 --- Diff: service/src/main/java/org/apache/griffin/core/job/JobServiceImpl.java --- @@ -158,17 +162,22 @@ public JobServiceImpl() {

[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 I revise test case, can you review it again ---

[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 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 toyboxman commented on the issue: https://github.com/apache/incubator-griffin/pull/444 probably we can think about labeling something during onEvent implementation in testcase ---

[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 spring beans framework introspect introspect top-level beans, that is why List not wired on. I agree with @gavlyukovskiy getbean by name is not as good as getbean by type. But

[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 hoo

[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 def

[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. ---