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

2018-10-30 Thread chemikadze
Github user chemikadze commented on a diff in the pull request:

https://github.com/apache/incubator-griffin/pull/444#discussion_r229565301
  
--- Diff: 
service/src/test/java/org/apache/griffin/core/job/EventServiceTest.java ---
@@ -0,0 +1,69 @@
+package org.apache.griffin.core.job;
+
+import static 
org.apache.griffin.core.util.EntityHelper.createGriffinMeasure;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.griffin.core.exception.GriffinException;
+import org.apache.griffin.core.integration.GriffinEvent;
+import org.apache.griffin.core.integration.GriffinHook;
+import org.apache.griffin.core.integration.JobEvent;
+import org.apache.griffin.core.job.entity.BatchJob;
+import org.apache.griffin.core.job.entity.JobDataSegment;
+import org.apache.griffin.core.measure.entity.GriffinMeasure;
+import org.apache.griffin.core.measure.entity.Measure;
+import org.apache.griffin.core.util.EntityHelper;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import 
org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager;
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.util.Assert;
+
+@RunWith(SpringRunner.class)
+//@TestConfiguration
+//@AutoConfigureTestEntityManager
+@DataJpaTest
+@ComponentScan("org.apache.griffin.core")
+//@SpringBootTest
+//@ContextConfiguration(classes = {JobServiceImpl.class, 
SchedulerFactoryBean.class, JobInstanceRepo.class})
+public class EventServiceTest {
+@Autowired
+private JobService jobService;
+
+@Autowired
+private TestEntityManager entityManager;
+
+@Before
+public void setup() throws Exception {
+Class.forName("org.apache.griffin.core.integration.JobEventHook");
+entityManager.clear();
+entityManager.flush();
+setEntityManager();
+}
+
+@Test
+public void testAddJobEvent() throws Exception {
--- End diff --

This test is just verifying that code compiles, and when hook runs, 
exception is logged. Is that right?


---


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

2018-10-30 Thread chemikadze
Github user chemikadze commented on a diff in the pull request:

https://github.com/apache/incubator-griffin/pull/444#discussion_r229564055
  
--- Diff: 
service/src/main/java/org/apache/griffin/core/integration/JobEvent.java ---
@@ -0,0 +1,21 @@
+package org.apache.griffin.core.integration;
+
+import org.apache.griffin.core.job.entity.AbstractJob;
+
+public class JobEvent implements GriffinEvent {
--- End diff --

Should we classify it by BeforeJobAdd / AfterJobAdd / BeforeJobDelete / 
AfterJobDelete?


---


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

2018-10-30 Thread chemikadze
Github user chemikadze commented on a diff in the pull request:

https://github.com/apache/incubator-griffin/pull/444#discussion_r229565395
  
--- Diff: 
service/src/main/java/org/apache/griffin/core/integration/GriffinEvent.java ---
@@ -0,0 +1,44 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+package org.apache.griffin.core.integration;
+
+/**
+ * A semantic event which indicates that a griffin-defined action occurred.
+ * This high-level event is generated by an action (such as an
+ * addJob) when the task-specific action occurs.
+ * The event is passed to every GriffinHook object
+ * that registered to receive such events using configuration.
+ *
+ * @author Eugene Liu
+ * @since 0.3
+ */
+public interface GriffinEvent {
+/**
+ * @return concrete event type
+ */
+String getType();
--- End diff --

should it default method on interface be then?


---


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

2018-10-30 Thread chemikadze
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.


---


[jira] [Commented] (GRIFFIN-208) Job status is SUCCESS even if some stages have failed

2018-10-30 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/GRIFFIN-208?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16668266#comment-16668266
 ] 

ASF GitHub Bot commented on GRIFFIN-208:


Github user asfgit closed the pull request at:

https://github.com/apache/incubator-griffin/pull/448


> Job status is SUCCESS even if some stages have failed
> -
>
> Key: GRIFFIN-208
> URL: https://issues.apache.org/jira/browse/GRIFFIN-208
> Project: Griffin (Incubating)
>  Issue Type: Bug
>Reporter: Nikolay Sokolov
>Priority: Major
>
> When some steps (MetricWrite or SparkSql, for example) fail, errors are just 
> logged, but not reported as part of job status. Symptoms:
> {code:none} 
> 18/10/22 17:17:58 ERROR transform.SparkSqlTransformStep: run spark sql [  
> ] error: ...
> {code}
> YarnApplicationState: FINISHED
> FinalStatus Reported by AM:   SUCCEEDED



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] incubator-griffin pull request #448: [GRIFFIN-208] log exception details whe...

2018-10-30 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-griffin/pull/448


---