[GitHub] incubator-griffin pull request #456: [GRIFFIN-213] Custom connector support
Github user gavlyukovskiy commented on a diff in the pull request:
https://github.com/apache/incubator-griffin/pull/456#discussion_r234425707
--- Diff:
measure/src/main/scala/org/apache/griffin/measure/datasource/connector/DataConnectorFactory.scala
---
@@ -84,6 +87,26 @@ object DataConnectorFactory extends Loggable {
}
}
+ private def getCustomConnector(session: SparkSession,
+ context: StreamingContext,
+ param: DataConnectorParam,
+ storage: TimestampStorage,
+ maybeClient:
Option[StreamingCacheClient]): DataConnector = {
+val className = param.getConfig("class").asInstanceOf[String]
+val cls = Class.forName(className)
+if (classOf[BatchDataConnector].isAssignableFrom(cls)) {
+ val ctx = BatchDataConnectorContext(session, param, storage)
+ val meth = cls.getDeclaredMethod("apply",
classOf[BatchDataConnectorContext])
+ meth.invoke(null, ctx).asInstanceOf[BatchDataConnector]
+} else if (classOf[StreamingDataConnector].isAssignableFrom(cls)) {
+ val ctx = StreamingDataConnectorContext(session, context, param,
storage, maybeClient)
+ val meth = cls.getDeclaredMethod("apply",
classOf[StreamingDataConnectorContext])
+ meth.invoke(null, ctx).asInstanceOf[StreamingDataConnector]
+} else {
+ throw new ClassCastException("")
--- End diff --
It would be nice to have here message that custom connector class must
extend `BatchDataConnector` or `StreamingDataConnector`
---
[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 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 hooks in the context, but use only ones that explicitly set in property > Do we really need that much hooks? We are deploying griffin with custom set of plugins bundled in jar, we will probably define few custom hooks in that jar so that they will be available in griffin context ---
[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.griffin.core.event;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Configuration
+public class GriffinEventListeners {
+private static final Logger LOGGER = LoggerFactory
+.getLogger(GriffinEventListeners.class);
+
+@Autowired
+private ApplicationContext context;
+@Value("#{'${internal.event.listeners}'.split(',')}")
+private List listeners;
+
+@Bean
+public List getListenersRegistered() {
--- End diff --
I see that you getting beans by names from the context, am I right that
you're trying to declare only beans that user has specified in the property? I
think it will not work, because `@Autowired List eventListeners`
will always inject all beans of given type from the context.
For example if you do:
```
@Bean
public static List list() {
return Arrays.asList(
new GriffinHook("1"),
new GriffinHook("2")
);
}
@Bean
public static GriffinHook hook3() {
return new GriffinHook("3");
}
@Autowired
private final List griffinHooks;
```
in this case spring will try to inject list of beans of given type - list
containing only `hook3`.
Instead I would propose to remove this bean declaration and make changes in
`GriffinEventManager` to get all beans from context and use only beans
specified in the config:
```
public class GriffinEventManager {
@Autowired
private ApplicationContext applicationContext;
@Value("#{'${internal.event.listeners}'.split(',')}")
private Set enabledListeners;
private List eventListeners;
@PostConstruct
void initializeListeners() {
List eventListeners = new ArrayList<>();
applicationContext.getBeansOfType(GriffinHook.class)
.forEach((beanName, listener) -> {
if (enabledListeners.contains(beanName)) {
eventListeners.add(listener);
}
});
this.eventListeners = eventListeners;
}
...
}
```
---
[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 for testing purpose. ---
[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_r231008571 --- Diff: service/src/main/java/org/apache/griffin/core/event/GriffinEventListeners.java --- @@ -0,0 +1,51 @@ +package org.apache.griffin.core.event; + +import java.util.ArrayList; +import java.util.List; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Configuration +@ConfigurationProperties(prefix = "internal.event") --- End diff -- I'm not sure you need it here, this will map all class fields (that have setters and getters) to properties. ---
[GitHub] incubator-griffin pull request #446: [GRIFFIN-203] "Plaintext mode" for meas...
Github user gavlyukovskiy commented on a diff in the pull request:
https://github.com/apache/incubator-griffin/pull/446#discussion_r228039612
--- Diff: ui/angular/src/app/measure/create-measure/raw/raw.component.ts ---
@@ -0,0 +1,146 @@
+/*
+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.
+*/
+import {Component, OnInit} from "@angular/core";
+import {ServiceService} from "../../../service/service.service";
+import {TREE_ACTIONS, ITreeOptions} from "angular-tree-component";
+import {ToasterService} from "angular2-toaster";
+import * as $ from "jquery";
+import {HttpClient} from "@angular/common/http";
+import {ActivatedRoute, Router} from "@angular/router";
+import {AfterViewChecked, ElementRef} from "@angular/core";
+import {MeasureFormatService, Format} from
"../../../service/measure-format.service";
+
+@Component({
+ selector: "app-raw",
+ templateUrl: "./raw.component.html",
+ providers: [ServiceService, MeasureFormatService],
+ styleUrls: ["./raw.component.css"]
+})
+export class RawComponent implements AfterViewChecked {
+
+ constructor(
+private elementRef: ElementRef,
+private toasterService: ToasterService,
+private measureFormatService: MeasureFormatService,
+private http: HttpClient,
+private router: Router,
+public serviceService: ServiceService
+ ) {
+ }
+
+ data = "";
+ valid = false;
+ Format: typeof Format = Format;
+ format: Format;
+ createResult: any;
+ public visible = false;
+ public visibleAnimate = false;
+
+ public hide(): void {
+this.visibleAnimate = false;
+setTimeout(() => (this.visible = false), 300);
+$("#save").removeAttr("disabled");
+ }
+
+ public onContainerClicked(event: MouseEvent): void {
+if ((event.target).classList.contains("modal")) {
+ this.hide();
+}
+ }
+
+ onResize(event) {
+this.resizeWindow();
+ }
+
+ resizeWindow() {
+var stepSelection = ".formStep";
+$(stepSelection).css({
+ height: window.innerHeight - $(stepSelection).offset().top
+});
+$("fieldset").height(
+ $(stepSelection).height() -
+ $(stepSelection + ">.stepDesc").height() -
+ $(".btn-container").height() -
+ 130
+);
+$(".y-scrollable").css({
+ height: $("fieldset").height()
+});
+ }
+
+ submit(form) {
+if (!form.valid) {
+ this.toasterService.pop(
+"error",
+"Error!",
+"please complete the form in this step before proceeding"
+ );
+ return false;
+}
+this.visible = true;
+setTimeout(() => (this.visibleAnimate = true), 100);
+ }
+
+ save() {
+let measure2Save = this.measureFormatService.parse(this.data,
this.format);
+console.log(measure2Save);
+let addModels = this.serviceService.config.uri.addModels;
+$("#save").attr("disabled", "true");
+this.http.post(addModels, measure2Save).subscribe(
+ data => {
+this.createResult = data;
+this.hide();
+this.router.navigate(["/measures"]);
+ },
+ err => {
+let response = JSON.parse(err.error);
+if (response.code === '40901') {
+ this.toasterService.pop("error", "Error!", "Measure name already
exists!");
+} else {
+ this.toasterService.pop("error", "Error!", response.message);
+}
+console.log("Error when creating measure");
+ }
+);
+ }
+
+ onInputChange() {
--- End diff --
As I can see it's done on every change, but is not noticeable on my laptop.
Not sure if it will affect others, but anyway it's much easier to edit measure
in normal editor and just copy-past it here :)
---
[GitHub] incubator-griffin pull request #446: [GRIFFIN-203] "Plaintext mode" for meas...
Github user gavlyukovskiy commented on a diff in the pull request:
https://github.com/apache/incubator-griffin/pull/446#discussion_r228037408
--- Diff: ui/angular/src/app/measure/create-measure/raw/raw.component.ts ---
@@ -0,0 +1,180 @@
+/*
+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.
+*/
+import {Component, OnInit} from "@angular/core";
+import {ServiceService} from "../../../service/service.service";
+import {TREE_ACTIONS, ITreeOptions} from "angular-tree-component";
+import {ToasterService} from "angular2-toaster";
+import * as $ from "jquery";
+import {HttpClient} from "@angular/common/http";
+import {ActivatedRoute, Router} from "@angular/router";
+import {AfterViewChecked, ElementRef} from "@angular/core";
+import {MeasureFormatService, Format} from
"../../../service/measure-format.service";
+
+@Component({
+ selector: "app-raw",
+ templateUrl: "./raw.component.html",
+ providers: [ServiceService, MeasureFormatService],
+ styleUrls: ["./raw.component.css"]
+})
+export class RawComponent implements AfterViewChecked, OnInit {
+
+ constructor(
+private elementRef: ElementRef,
+private toasterService: ToasterService,
+private measureFormatService: MeasureFormatService,
+private http: HttpClient,
+private router: Router,
+public serviceService: ServiceService
+ ) {
+ }
+
+ data = "";
+ valid = false;
+ Format: typeof Format = Format;
+ format: Format;
+ createResult: any;
+ public visible = false;
+ public visibleAnimate = false;
+
+ public hide(): void {
+this.visibleAnimate = false;
+setTimeout(() => (this.visible = false), 300);
+$("#save").removeAttr("disabled");
+ }
+
+ public onContainerClicked(event: MouseEvent): void {
+if ((event.target).classList.contains("modal")) {
+ this.hide();
+}
+ }
+
+ onResize(event) {
+this.resizeWindow();
+ }
+
+ resizeWindow() {
+var stepSelection = ".formStep";
+$(stepSelection).css({
+ height: window.innerHeight - $(stepSelection).offset().top
+});
+$("fieldset").height(
+ $(stepSelection).height() -
+ $(stepSelection + ">.stepDesc").height() -
+ $(".btn-container").height() -
+ 130
+);
+$(".y-scrollable").css({
+ height: $("fieldset").height()
+});
+ }
+
+ submit(form) {
+if (!form.valid) {
+ this.toasterService.pop(
+"error",
+"Error!",
+"please complete the form in this step before proceeding"
+ );
+ return false;
+}
+this.visible = true;
+setTimeout(() => (this.visibleAnimate = true), 100);
+ }
+
+ save() {
+let measure2Save = this.measureFormatService.parse(this.data,
this.format);
+console.log(measure2Save);
+let addModels = this.serviceService.config.uri.addModels;
+$("#save").attr("disabled", "true");
+this.http.post(addModels, measure2Save).subscribe(
+ data => {
+this.createResult = data;
+this.hide();
+this.router.navigate(["/measures"]);
+ },
+ err => {
+let response = JSON.parse(err.error);
+if (response.code === '40901') {
+ this.toasterService.pop("error", "Error!", "Measure name already
exists!");
+} else {
+ this.toasterService.pop("error", "Error!", "Measure is not
valid");
--- End diff --
fixed
---
[GitHub] incubator-griffin pull request #446: [GRIFFIN-203] "Plaintext mode" for meas...
Github user gavlyukovskiy commented on a diff in the pull request:
https://github.com/apache/incubator-griffin/pull/446#discussion_r228037399
--- Diff: ui/angular/src/app/measure/create-measure/raw/raw.component.ts ---
@@ -0,0 +1,180 @@
+/*
+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.
+*/
+import {Component, OnInit} from "@angular/core";
+import {ServiceService} from "../../../service/service.service";
+import {TREE_ACTIONS, ITreeOptions} from "angular-tree-component";
+import {ToasterService} from "angular2-toaster";
+import * as $ from "jquery";
+import {HttpClient} from "@angular/common/http";
+import {ActivatedRoute, Router} from "@angular/router";
+import {AfterViewChecked, ElementRef} from "@angular/core";
+import {MeasureFormatService, Format} from
"../../../service/measure-format.service";
+
+@Component({
+ selector: "app-raw",
+ templateUrl: "./raw.component.html",
+ providers: [ServiceService, MeasureFormatService],
+ styleUrls: ["./raw.component.css"]
+})
+export class RawComponent implements AfterViewChecked, OnInit {
+
+ constructor(
+private elementRef: ElementRef,
+private toasterService: ToasterService,
+private measureFormatService: MeasureFormatService,
+private http: HttpClient,
+private router: Router,
+public serviceService: ServiceService
+ ) {
+ }
+
+ data = "";
+ valid = false;
+ Format: typeof Format = Format;
+ format: Format;
+ createResult: any;
+ public visible = false;
+ public visibleAnimate = false;
+
+ public hide(): void {
+this.visibleAnimate = false;
+setTimeout(() => (this.visible = false), 300);
+$("#save").removeAttr("disabled");
+ }
+
+ public onContainerClicked(event: MouseEvent): void {
+if ((event.target).classList.contains("modal")) {
+ this.hide();
+}
+ }
+
+ onResize(event) {
+this.resizeWindow();
+ }
+
+ resizeWindow() {
+var stepSelection = ".formStep";
+$(stepSelection).css({
+ height: window.innerHeight - $(stepSelection).offset().top
+});
+$("fieldset").height(
+ $(stepSelection).height() -
+ $(stepSelection + ">.stepDesc").height() -
+ $(".btn-container").height() -
+ 130
+);
+$(".y-scrollable").css({
+ height: $("fieldset").height()
+});
+ }
+
+ submit(form) {
+if (!form.valid) {
+ this.toasterService.pop(
+"error",
+"Error!",
+"please complete the form in this step before proceeding"
+ );
+ return false;
+}
+this.visible = true;
+setTimeout(() => (this.visibleAnimate = true), 100);
+ }
+
+ save() {
+let measure2Save = this.measureFormatService.parse(this.data,
this.format);
+console.log(measure2Save);
+let addModels = this.serviceService.config.uri.addModels;
+$("#save").attr("disabled", "true");
+this.http.post(addModels, measure2Save).subscribe(
+ data => {
+this.createResult = data;
+this.hide();
+this.router.navigate(["/measures"]);
+ },
+ err => {
+let response = JSON.parse(err.error);
+if (response.code === '40901') {
+ this.toasterService.pop("error", "Error!", "Measure name already
exists!");
+} else {
+ this.toasterService.pop("error", "Error!", "Measure is not
valid");
+}
+console.log("Error when creating measure");
+ }
+);
+ }
[GitHub] incubator-griffin pull request #446: [GRIFFIN-203] "Plaintext mode" for meas...
Github user gavlyukovskiy commented on a diff in the pull request: https://github.com/apache/incubator-griffin/pull/446#discussion_r228036531 --- Diff: ui/angular/src/app/measure/create-measure/raw/raw.component.html --- @@ -0,0 +1,100 @@ + + + +Create Measure + + + + + + + + +1 + + Configuration + + + + + +Please setup the measure required information + + + + +Required Information + + + + + + Raw measure: + + + +JSON +YAML + + + + + +Measure is not valid + + + + + + After submitted, please go to "Measures" + to check the measure status + + + + + + + + Submit + + + + + + --- End diff -- I just didn't captured whole window, I've just checked and it's centered. ---
[GitHub] incubator-griffin pull request #446: [GRIFFIN-203] "Plaintext mode" for meas...
GitHub user gavlyukovskiy opened a pull request: https://github.com/apache/incubator-griffin/pull/446 [GRIFFIN-203] "Plaintext mode" for measure creation Created json/yaml measure creation flow:  Added ability to view measure as yaml:  You can merge this pull request into a Git repository by running: $ git pull https://github.com/gavlyukovskiy/incubator-griffin raw-measures Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-griffin/pull/446.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #446 commit e07a7e169b18f67fd5661ab085524c3bf9206c8b Author: Arthur Gavlyukovskiy Date: 2018-10-25T04:53:01Z [GRIFFIN-203] "Plaintext mode" for measure creation Created json/yaml measure creation flow Added ability to view measure as yaml ---
[GitHub] incubator-griffin pull request #435: [GRIFFIN-206] fix job timezone when cre...
Github user gavlyukovskiy commented on a diff in the pull request:
https://github.com/apache/incubator-griffin/pull/435#discussion_r224560857
--- Diff:
service/src/test/java/org/apache/griffin/core/util/TimeUtilTest.java ---
@@ -110,4 +111,36 @@ public void testFormatWithIllegalException() {
TimeUtil.format(format, time, TimeZone.getTimeZone(timeZone));
}
+@Test
+public void testGetTimeZone() {
+HashMap tests = new HashMap<>();
+tests.put("", TimeZone.getDefault().getID());
+// standard cases
+tests.put("GMT", "GMT");
+tests.put("GMT+1", "GMT+01:00");
+tests.put("GMT+1:00", "GMT+01:00");
+tests.put("GMT+01:00", "GMT+01:00");
+tests.put("GMT-1", "GMT-01:00");
+tests.put("GMT-1:00", "GMT-01:00");
+tests.put("GMT-01:00", "GMT-01:00");
+// values pushed by UI for jobs
+tests.put("GMT1", "GMT");
+tests.put("GMT1:00", "GMT");
+tests.put("GMT01:00", "GMT");
+// values generated by UI for datasets in a past
+tests.put("UTC1", "GMT");
+tests.put("UTC1:00", "GMT");
+tests.put("UTC01:00", "GMT");
+tests.put("UTC-1", "GMT");
+tests.put("UTC-1:00", "GMT");
+tests.put("UTC-01:00", "GMT");
+for (HashMap.Entry e: tests.entrySet()) {
--- End diff --
```
tests.forEach((key, value) -> {...})
```
Looks even better :)
---
