[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
sv2000 commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship
cleaning and bumping up ORC version
URL: https://github.com/apache/incubator-gobblin/pull/2784#discussion_r339284617
##
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixJobLauncher.java
##
@@ -381,7 +381,8 @@ public void launchJob(@Nullable JobListener jobListener)
}
/**
- * Add a single {@link WorkUnit} (flattened).
+ * Add a single {@link WorkUnit} (flattened) to persisted storage so that
worker could fetch that based on information
Review comment:
Nit: "persisted" -> "persistent". "could" -> "can".
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
sv2000 commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship
cleaning and bumping up ORC version
URL: https://github.com/apache/incubator-gobblin/pull/2784#discussion_r339284718
##
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/StateStoreBasedWatermarkStorage.java
##
@@ -50,7 +50,12 @@
public static final String WATERMARK_STORAGE_TYPE_KEY
="streaming.watermarkStateStore.type";
public static final String WATERMARK_STORAGE_TYPE_DEFAULT ="zk";
public static final String
WATERMARK_STORAGE_CONFIG_PREFIX="streaming.watermarkStateStore.config.";
- private static final String WATERMARK_STORAGE_PREFIX="streamingWatermarks:";
+
+ /**
+ * The prefix need to be fitting with different storage layer. e.g. some of
characters won't be allowed in URI and
Review comment:
A bit of word-smithing: "A watermark prefix that is compatible with
different watermark storage implementations. As such, this prefix should not
include any characters disallowed in a {@link URI}."
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com…
jhsenjaliya commented on a change in pull request #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com… URL: https://github.com/apache/incubator-gobblin/pull/2782#discussion_r339321774 ## File path: conf/standalone/log4j.xml ## @@ -3,7 +3,7 @@ - + Review comment: file name here is mode name, any reason to change it to gobblin-current ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com…
jhsenjaliya commented on issue #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com… URL: https://github.com/apache/incubator-gobblin/pull/2782#issuecomment-546653477 LGTM, other than a file name comment. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add
MySQL and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339321805
##
File path: bin/gobblin.sh
##
@@ -105,7 +107,7 @@ function print_gobblin_cli_usage() {
job-store-schema-managerDatabase job history store schema
manager
gobblin-classpath shows the constructed gobblin
classpath"
echo ""
-echo "--conf-dir Gobblon config path.
default is '\$GOBBLIN_HOME/conf/'."
+echo "--conf-dir Gobblin config path.
default is '\$GOBBLIN_HOME/conf/'."
Review comment:
lol, Thanks for taking care of such spelling mistakes..
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add
MySQL and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339321965
##
File path: bin/gobblin.sh
##
@@ -135,6 +137,7 @@ function print_gobblin_service_usage() {
echo "--job-conf-file Only for mapreduce mode:
configuration file for the job to run"
echo "--helpDisplay this help."
echo "--verbose Display full command used
to start the process."
+echo "--argsAdditional -D args"
Review comment:
"args" can be confusing since its basically a java arguments
btw isnt this covered as jvmopts ? since any strings passed as jvmopts will
be placed in java command.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add
MySQL and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339321995
##
File path: gobblin-docker/gobblin-service/alpine-gaas-latest/entrypoint.sh
##
@@ -15,8 +15,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+
GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)"
+ARG_FLAG=""
+# Treat the config env args differently as they need to be combined together
in 1 arg
+CONFIG_ARGS=""
+shopt -s nocasematch
+for i in "$@"
+do
+case "$1" in
+--args)
+ARG_FLAG="--args"
+CONFIG_ARGS="$2"
+shift
+;;
+esac
+shift
+done
-./bin/gobblin.sh service gobblin-as-service start
alpine-gaas-latest_gobblin-standalone
-busybox tail -F $GOBBLIN_HOME/logs/gobblin-as-service.out
+./bin/gobblin.sh service gobblin-as-service start ${ARG_FLAG} "${CONFIG_ARGS}"
$@
+busybox tail -F $GOBBLIN_HOME/logs/gobblin-as-service.out
$GOBBLIN_HOME/logs/gobblin-as-service.err
Review comment:
does busybox becomes requirements/dependencies for GAAS ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
autumnust commented on a change in pull request #2784:
[GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
URL: https://github.com/apache/incubator-gobblin/pull/2784#discussion_r339371421
##
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixJobLauncher.java
##
@@ -381,7 +381,8 @@ public void launchJob(@Nullable JobListener jobListener)
}
/**
- * Add a single {@link WorkUnit} (flattened).
+ * Add a single {@link WorkUnit} (flattened) to persisted storage so that
worker could fetch that based on information
Review comment:
Thanks, addressed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
autumnust commented on a change in pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version URL: https://github.com/apache/incubator-gobblin/pull/2784#discussion_r339371471 ## File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/StateStoreBasedWatermarkStorage.java ## @@ -50,7 +50,12 @@ public static final String WATERMARK_STORAGE_TYPE_KEY ="streaming.watermarkStateStore.type"; public static final String WATERMARK_STORAGE_TYPE_DEFAULT ="zk"; public static final String WATERMARK_STORAGE_CONFIG_PREFIX="streaming.watermarkStateStore.config."; - private static final String WATERMARK_STORAGE_PREFIX="streamingWatermarks:"; + + /** + * The prefix need to be fitting with different storage layer. e.g. some of characters won't be allowed in URI and Review comment: Thanks, addressed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
codecov-io commented on issue #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version URL: https://github.com/apache/incubator-gobblin/pull/2784#issuecomment-546750094 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=h1) Report > Merging [#2784](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/70937f67a80e68a2a57b5aae3fe72e3d0fa087fd?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2784 +/- ## - Coverage 45.32% 45.32% -0.01% + Complexity 8860 8859 -1 Files 1894 1894 Lines 7087270874 +2 Branches 7793 7794 +1 Hits 3212632126 + Misses3578535783 -2 - Partials 2961 2965 +4 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.42% <ø> (-1.39%)` | `27 <0> (-1)` | | | [...he/gobblin/cluster/TaskRunnerSuiteThreadModel.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvVGFza1J1bm5lclN1aXRlVGhyZWFkTW9kZWwuamF2YQ==) | `100% <ø> (ø)` | `5 <0> (ø)` | :arrow_down: | | [...bblin/runtime/StateStoreBasedWatermarkStorage.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvU3RhdGVTdG9yZUJhc2VkV2F0ZXJtYXJrU3RvcmFnZS5qYXZh) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...pache/gobblin/cluster/GobblinHelixJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4Sm9iTGF1bmNoZXIuamF2YQ==) | `81.81% <ø> (ø)` | `26 <0> (ø)` | :arrow_down: | | [...rg/apache/gobblin/yarn/GobblinYarnAppLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi15YXJuL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3lhcm4vR29iYmxpbllhcm5BcHBMYXVuY2hlci5qYXZh) | `20.05% <0%> (-0.11%)` | `7 <0> (ø)` | | | [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `40% <0%> (-20%)` | `2% <0%> (-1%)` | | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0%> (-14.29%)` | `3% <0%> (-1%)` | | | [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0%> (-0.94%)` | `24% <0%> (ø)` | | | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2784/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `39.25% <0%> (+6.54%)` | `13% <0%> (+2%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=footer). Last update [70937f6...4486a60](https://codecov.io/gh/apache/incubator-gobblin/pull/2784?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). --
[GitHub] [incubator-gobblin] asfgit closed pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version
asfgit closed pull request #2784: [GOBBLIN-928]Craftsmanship cleaning and bumping up ORC version URL: https://github.com/apache/incubator-gobblin/pull/2784 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on issue #2780: [GOBBLIN-924]Get rid of orc.schema.literal in ORC-ingestion and registration
autumnust commented on issue #2780: [GOBBLIN-924]Get rid of orc.schema.literal in ORC-ingestion and registration URL: https://github.com/apache/incubator-gobblin/pull/2780#issuecomment-547024231 > @autumnust I use a pull command which seems introduce all the changes which have been committed during last two days. Just select the last commit should be able to see my change. Sorry for the inconvenience ^^ You can try `pull --rebase` and force a push to your own remote. Should be able to create a clean diff. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com…
Will-Lo commented on a change in pull request #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com… URL: https://github.com/apache/incubator-gobblin/pull/2782#discussion_r339700268 ## File path: conf/standalone/log4j.xml ## @@ -3,7 +3,7 @@ - + Review comment: Ah good point I'll just change it back, I think I wanted it to match prior to the script change but I realize now there's no real point of breaking naming convention This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339729627
##
File path: bin/gobblin.sh
##
@@ -105,7 +107,7 @@ function print_gobblin_cli_usage() {
job-store-schema-managerDatabase job history store schema
manager
gobblin-classpath shows the constructed gobblin
classpath"
echo ""
-echo "--conf-dir Gobblon config path.
default is '\$GOBBLIN_HOME/conf/'."
+echo "--conf-dir Gobblin config path.
default is '\$GOBBLIN_HOME/conf/'."
Review comment:
Haha no probs
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339764542
##
File path: bin/gobblin.sh
##
@@ -135,6 +137,7 @@ function print_gobblin_service_usage() {
echo "--job-conf-file Only for mapreduce mode:
configuration file for the job to run"
echo "--helpDisplay this help."
echo "--verbose Display full command used
to start the process."
+echo "--argsAdditional -D args"
Review comment:
Ah true I missed that, thanks for pointing it out!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339765341
##
File path: gobblin-docker/gobblin-service/alpine-gaas-latest/entrypoint.sh
##
@@ -15,8 +15,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+
GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)"
+ARG_FLAG=""
+# Treat the config env args differently as they need to be combined together
in 1 arg
+CONFIG_ARGS=""
+shopt -s nocasematch
+for i in "$@"
+do
+case "$1" in
+--args)
+ARG_FLAG="--args"
+CONFIG_ARGS="$2"
+shift
+;;
+esac
+shift
+done
-./bin/gobblin.sh service gobblin-as-service start
alpine-gaas-latest_gobblin-standalone
-busybox tail -F $GOBBLIN_HOME/logs/gobblin-as-service.out
+./bin/gobblin.sh service gobblin-as-service start ${ARG_FLAG} "${CONFIG_ARGS}"
$@
+busybox tail -F $GOBBLIN_HOME/logs/gobblin-as-service.out
$GOBBLIN_HOME/logs/gobblin-as-service.err
Review comment:
It was to workaround a bug with tailing files in kubernetes, but busybox
becomes unneeded in https://github.com/apache/incubator-gobblin/pull/2782
Regardless, this file is for executing GAAS in a dockerized context so the
dependencies should be all within the container
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r339765341
##
File path: gobblin-docker/gobblin-service/alpine-gaas-latest/entrypoint.sh
##
@@ -15,8 +15,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+
GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)"
+ARG_FLAG=""
+# Treat the config env args differently as they need to be combined together
in 1 arg
+CONFIG_ARGS=""
+shopt -s nocasematch
+for i in "$@"
+do
+case "$1" in
+--args)
+ARG_FLAG="--args"
+CONFIG_ARGS="$2"
+shift
+;;
+esac
+shift
+done
-./bin/gobblin.sh service gobblin-as-service start
alpine-gaas-latest_gobblin-standalone
-busybox tail -F $GOBBLIN_HOME/logs/gobblin-as-service.out
+./bin/gobblin.sh service gobblin-as-service start ${ARG_FLAG} "${CONFIG_ARGS}"
$@
+busybox tail -F $GOBBLIN_HOME/logs/gobblin-as-service.out
$GOBBLIN_HOME/logs/gobblin-as-service.err
Review comment:
It was to workaround a bug with tailing files in kubernetes, but busybox
becomes unneeded in https://github.com/apache/incubator-gobblin/pulls
Regardless, this file is for executing GAAS in a dockerized context so the
dependencies should be all within the container
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com…
codecov-io commented on issue #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com… URL: https://github.com/apache/incubator-gobblin/pull/2782#issuecomment-547126016 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=h1) Report > Merging [#2782](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/238f2e1cf8cc1be523cef47240c855cd596ea260?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2782 +/- ## - Coverage 45.33% 45.32% -0.01% + Complexity 8861 8859 -2 Files 1894 1894 Lines 7087470874 Branches 7794 7794 - Hits 3213432127 -7 - Misses3577735782 +5 - Partials 2963 2965 +2 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2782/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `40% <0%> (-20%)` | `2% <0%> (-1%)` | | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2782/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0%> (-14.29%)` | `3% <0%> (-1%)` | | | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2782/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `35.51% <0%> (-3.74%)` | `12% <0%> (-1%)` | | | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2782/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `64.81% <0%> (-0.47%)` | `28% <0%> (ø)` | | | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2782/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `79.68% <0%> (+1.56%)` | `16% <0%> (+1%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=footer). Last update [238f2e1...d74ff76](https://codecov.io/gh/apache/incubator-gobblin/pull/2782?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] asfgit closed pull request #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com…
asfgit closed pull request #2782: [GOBBLIN-925] Create option to log outputs to console, fix docker-com… URL: https://github.com/apache/incubator-gobblin/pull/2782 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#issuecomment-545699470 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=h1) Report > Merging [#2781](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/238f2e1cf8cc1be523cef47240c855cd596ea260?src=pr&el=desc) will **increase** coverage by `<.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2781 +/- ## + Coverage 45.33% 45.34% +<.01% - Complexity 8861 8863 +2 Files 1894 1894 Lines 7087470874 Branches 7794 7794 + Hits 3213432135 +1 Misses3577735777 + Partials 2963 2962 -1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...lin/elasticsearch/writer/FutureCallbackHolder.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3dyaXRlci9GdXR1cmVDYWxsYmFja0hvbGRlci5qYXZh) | `61.42% <0%> (-1.43%)` | `4% <0%> (ø)` | | | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `64.44% <0%> (+1.11%)` | `16% <0%> (+1%)` | :arrow_up: | | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `79.68% <0%> (+1.56%)` | `16% <0%> (+1%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=footer). Last update [238f2e1...8a91f9d](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#issuecomment-545699470 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=h1) Report > Merging [#2781](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d9e4f9a39f56c845d82fceb1a0c410655bca09b0?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2781 +/- ## + Coverage 45.32% 45.34% +0.01% Complexity 8858 8858 Files 1894 1894 Lines 7087470874 Branches 7794 7794 + Hits 3212332136 +13 + Misses3578835775 -13 Partials 2963 2963 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../modules/scheduler/GobblinServiceJobScheduler.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zY2hlZHVsZXIvR29iYmxpblNlcnZpY2VKb2JTY2hlZHVsZXIuamF2YQ==) | `65.71% <0%> (+7.42%)` | `21% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=footer). Last update [d9e4f9a...085282a](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md: Typos
jhsenjaliya commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md: Typos URL: https://github.com/apache/incubator-gobblin/pull/2783#issuecomment-547231948 LGTM, @bstaudacher do you want to just run the spell check on all docs and fix it ? Also pls create JIRA ticket and mention that here. Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] bstaudacher commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md: Typos
bstaudacher commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md: Typos URL: https://github.com/apache/incubator-gobblin/pull/2783#issuecomment-547637326 I don't believe I can make a JIRA ticket, I went and poked around but it appears I don't have the option to do so. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2759: [GOBBLIN-905] Fixes issue where newly added jobs would crash in gobbl…
jhsenjaliya commented on issue #2759: [GOBBLIN-905] Fixes issue where newly added jobs would crash in gobbl… URL: https://github.com/apache/incubator-gobblin/pull/2759#issuecomment-547999820 Thanks William for fixing this, I just encountered this yesterday (GOBBLIN-935) and fixed it but after rebasing it I realize you have beat me to get to this bug first just couple weeks ago 👍 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340768889 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/application.yaml ## @@ -0,0 +1,110 @@ +# In the future, build the kubernetes cluster from the official docker account +# Also ensure that proper tagging/versioning is done i.e. remove :latest tag and instead use the digest of the container + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: 'gaas-deployment' + labels: +app: gaas-deployment +spec: + selector: +matchLabels: + app: gaas + replicas: 1 + template: +metadata: + name: 'gaas' + labels: +app: gaas +spec: + volumes: +- name: shared-jobs + persistentVolumeClaim: +claimName: shared-jobs-claim +- name: shared-template-catalogs + persistentVolumeClaim: +claimName: shared-template-catalogs-claim +- name: gaas-config + configMap: +name: gaas-config + containers: +- name: gobblin-service + image: will97/gobblin-as-a-service:latest + command: ["./bin/entrypoint.sh"] + args: ["--args", "-DmysqlCredentials.user=$(MYSQL_USERNAME) -DmysqlCredentials.password=$(MYSQL_PASSWORD)"] + env: +- name: MYSQL_USERNAME + valueFrom: +secretKeyRef: + name: mysql-credentials + key: username +- name: MYSQL_PASSWORD + valueFrom: +secretKeyRef: + name: mysql-credentials + key: password + volumeMounts: +- name: shared-jobs + mountPath: /tmp/gobblin-as-service/jobs +- name: shared-template-catalogs + mountPath: /tmp/templateCatalog +- name: gaas-config + mountPath: /home/gobblin/conf/gobblin-as-service/application.conf + subPath: gaas-application.conf + # dependency on mysql to be initialized before gaas can be initialized + initContainers: Review comment: This is interesting. Does k8s have a way to specify POD dependencies while booting up application? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340762257 ## File path: gobblin-docker/gobblin-service/alpine-gaas-latest/entrypoint.sh ## @@ -15,7 +15,23 @@ # See the License for the specific language governing permissions and # limitations under the License. # + GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)" +JVM_OPT_FLAG="" +# Treat the config env args differently as they need to be combined together in 1 arg +CONFIG_ARGS="" +shopt -s nocasematch +for i in "$@" +do +case "$1" in +--args) Review comment: Can we simplify this by passing all args starting with `-D`, without speciying `--args` for each one? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340766549 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/application.yaml ## @@ -0,0 +1,110 @@ +# In the future, build the kubernetes cluster from the official docker account +# Also ensure that proper tagging/versioning is done i.e. remove :latest tag and instead use the digest of the container + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: 'gaas-deployment' + labels: +app: gaas-deployment +spec: + selector: +matchLabels: + app: gaas + replicas: 1 + template: +metadata: + name: 'gaas' + labels: +app: gaas +spec: + volumes: +- name: shared-jobs + persistentVolumeClaim: +claimName: shared-jobs-claim +- name: shared-template-catalogs + persistentVolumeClaim: +claimName: shared-template-catalogs-claim +- name: gaas-config + configMap: +name: gaas-config + containers: +- name: gobblin-service + image: will97/gobblin-as-a-service:latest + command: ["./bin/entrypoint.sh"] + args: ["--args", "-DmysqlCredentials.user=$(MYSQL_USERNAME) -DmysqlCredentials.password=$(MYSQL_PASSWORD)"] + env: +- name: MYSQL_USERNAME + valueFrom: +secretKeyRef: + name: mysql-credentials + key: username +- name: MYSQL_PASSWORD + valueFrom: +secretKeyRef: + name: mysql-credentials + key: password + volumeMounts: +- name: shared-jobs + mountPath: /tmp/gobblin-as-service/jobs +- name: shared-template-catalogs Review comment: What information do `share-template-catalogs` and `gaas-config` mount? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340762532
##
File path: bin/gobblin.sh
##
@@ -400,7 +401,7 @@ function start() {
fi
# execute the command
-$JAVA_HOME/bin/java $GC_OPTS $JVM_OPTS -cp $GOBBLIN_CLASSPATH
$CLI_CLASS $CMD_PARAMS
+$JAVA_HOME/bin/java $CONFIG_ARGS $GC_OPTS $JVM_OPTS -cp
$GOBBLIN_CLASSPATH $CLI_CLASS $CMD_PARAMS
Review comment:
Is `CONFIG_ARGS` defined somewhere?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2755: [GOBBLIN-897] adds local FS spec executor to write jobs to a local dir
codecov-io edited a comment on issue #2755: [GOBBLIN-897] adds local FS spec executor to write jobs to a local dir URL: https://github.com/apache/incubator-gobblin/pull/2755#issuecomment-539170866 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=h1) Report > Merging [#2755](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d9e4f9a39f56c845d82fceb1a0c410655bca09b0?src=pr&el=desc) will **decrease** coverage by `0.01%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #2755 +/- ## === - Coverage 45.32% 45.3% -0.02% - Complexity 88588859 +1 === Files 18941896 +2 Lines 70874 70908 +34 Branches 77947797 +3 === + Hits 32123 32125 +2 - Misses35788 35819 +31 - Partials 29632964 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ime/spec\_executorInstance/LocalFsSpecExecutor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2755/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvc3BlY19leGVjdXRvckluc3RhbmNlL0xvY2FsRnNTcGVjRXhlY3V0b3IuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | | | [...ime/spec\_executorInstance/LocalFsSpecProducer.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2755/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvc3BlY19leGVjdXRvckluc3RhbmNlL0xvY2FsRnNTcGVjUHJvZHVjZXIuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2755/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0%> (-7.15%)` | `3% <0%> (ø)` | | | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2755/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `64.35% <0%> (-0.47%)` | `27% <0%> (-1%)` | | | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2755/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `64.44% <0%> (+1.11%)` | `16% <0%> (+1%)` | :arrow_up: | | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2755/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `35.51% <0%> (+2.8%)` | `12% <0%> (+1%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=footer). Last update [d9e4f9a...69942f9](https://codecov.io/gh/apache/incubator-gobblin/pull/2755?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340791157
##
File path: bin/gobblin.sh
##
@@ -400,7 +401,7 @@ function start() {
fi
# execute the command
-$JAVA_HOME/bin/java $GC_OPTS $JVM_OPTS -cp $GOBBLIN_CLASSPATH
$CLI_CLASS $CMD_PARAMS
+$JAVA_HOME/bin/java $CONFIG_ARGS $GC_OPTS $JVM_OPTS -cp
$GOBBLIN_CLASSPATH $CLI_CLASS $CMD_PARAMS
Review comment:
Oh I forgot to remove this, good catch
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340791157
##
File path: bin/gobblin.sh
##
@@ -400,7 +401,7 @@ function start() {
fi
# execute the command
-$JAVA_HOME/bin/java $GC_OPTS $JVM_OPTS -cp $GOBBLIN_CLASSPATH
$CLI_CLASS $CMD_PARAMS
+$JAVA_HOME/bin/java $CONFIG_ARGS $GC_OPTS $JVM_OPTS -cp
$GOBBLIN_CLASSPATH $CLI_CLASS $CMD_PARAMS
Review comment:
Oh I forgot to remove this from a previous comment that pointed out its not
needed, good catch
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340794003 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/application.yaml ## @@ -0,0 +1,110 @@ +# In the future, build the kubernetes cluster from the official docker account +# Also ensure that proper tagging/versioning is done i.e. remove :latest tag and instead use the digest of the container + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: 'gaas-deployment' + labels: +app: gaas-deployment +spec: + selector: +matchLabels: + app: gaas + replicas: 1 + template: +metadata: + name: 'gaas' + labels: +app: gaas +spec: + volumes: +- name: shared-jobs + persistentVolumeClaim: +claimName: shared-jobs-claim +- name: shared-template-catalogs + persistentVolumeClaim: +claimName: shared-template-catalogs-claim +- name: gaas-config + configMap: +name: gaas-config + containers: +- name: gobblin-service + image: will97/gobblin-as-a-service:latest + command: ["./bin/entrypoint.sh"] + args: ["--args", "-DmysqlCredentials.user=$(MYSQL_USERNAME) -DmysqlCredentials.password=$(MYSQL_PASSWORD)"] + env: +- name: MYSQL_USERNAME + valueFrom: +secretKeyRef: + name: mysql-credentials + key: username +- name: MYSQL_PASSWORD + valueFrom: +secretKeyRef: + name: mysql-credentials + key: password + volumeMounts: +- name: shared-jobs + mountPath: /tmp/gobblin-as-service/jobs +- name: shared-template-catalogs + mountPath: /tmp/templateCatalog +- name: gaas-config + mountPath: /home/gobblin/conf/gobblin-as-service/application.conf + subPath: gaas-application.conf + # dependency on mysql to be initialized before gaas can be initialized + initContainers: Review comment: Other features of K8s to help with dependencies between pods that I'm aware of besides `initContainers` is to define a custom `readiness probe` and `readiness probe` to help with applications that take longer to boot up, I think we could incorporate that in the future as well to have a more robust startup This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340794003 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/application.yaml ## @@ -0,0 +1,110 @@ +# In the future, build the kubernetes cluster from the official docker account +# Also ensure that proper tagging/versioning is done i.e. remove :latest tag and instead use the digest of the container + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: 'gaas-deployment' + labels: +app: gaas-deployment +spec: + selector: +matchLabels: + app: gaas + replicas: 1 + template: +metadata: + name: 'gaas' + labels: +app: gaas +spec: + volumes: +- name: shared-jobs + persistentVolumeClaim: +claimName: shared-jobs-claim +- name: shared-template-catalogs + persistentVolumeClaim: +claimName: shared-template-catalogs-claim +- name: gaas-config + configMap: +name: gaas-config + containers: +- name: gobblin-service + image: will97/gobblin-as-a-service:latest + command: ["./bin/entrypoint.sh"] + args: ["--args", "-DmysqlCredentials.user=$(MYSQL_USERNAME) -DmysqlCredentials.password=$(MYSQL_PASSWORD)"] + env: +- name: MYSQL_USERNAME + valueFrom: +secretKeyRef: + name: mysql-credentials + key: username +- name: MYSQL_PASSWORD + valueFrom: +secretKeyRef: + name: mysql-credentials + key: password + volumeMounts: +- name: shared-jobs + mountPath: /tmp/gobblin-as-service/jobs +- name: shared-template-catalogs + mountPath: /tmp/templateCatalog +- name: gaas-config + mountPath: /home/gobblin/conf/gobblin-as-service/application.conf + subPath: gaas-application.conf + # dependency on mysql to be initialized before gaas can be initialized + initContainers: Review comment: Other features of K8s to help with dependencies between pods that I'm aware of besides `initContainers` is to define a custom `readiness probe` and `startup probe` to help with applications that take longer to boot up, I think we could incorporate that in the future as well to have a more robust startup This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340796164 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/application.yaml ## @@ -0,0 +1,110 @@ +# In the future, build the kubernetes cluster from the official docker account +# Also ensure that proper tagging/versioning is done i.e. remove :latest tag and instead use the digest of the container + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: 'gaas-deployment' + labels: +app: gaas-deployment +spec: + selector: +matchLabels: + app: gaas + replicas: 1 + template: +metadata: + name: 'gaas' + labels: +app: gaas +spec: + volumes: +- name: shared-jobs + persistentVolumeClaim: +claimName: shared-jobs-claim +- name: shared-template-catalogs + persistentVolumeClaim: +claimName: shared-template-catalogs-claim +- name: gaas-config + configMap: +name: gaas-config + containers: +- name: gobblin-service + image: will97/gobblin-as-a-service:latest + command: ["./bin/entrypoint.sh"] + args: ["--args", "-DmysqlCredentials.user=$(MYSQL_USERNAME) -DmysqlCredentials.password=$(MYSQL_PASSWORD)"] + env: +- name: MYSQL_USERNAME + valueFrom: +secretKeyRef: + name: mysql-credentials + key: username +- name: MYSQL_PASSWORD + valueFrom: +secretKeyRef: + name: mysql-credentials + key: password + volumeMounts: +- name: shared-jobs + mountPath: /tmp/gobblin-as-service/jobs +- name: shared-template-catalogs Review comment: `I'll remove share-template-catalog`, I was hoping to use it to share default templates among pods but I determined a better way of doing that in a future PR using configMaps `gaas-config` is for the `.conf` file for GaaS, I use this to replace the one defined in the `conf` folder due a change in the configs to support MySQL This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340796164 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/application.yaml ## @@ -0,0 +1,110 @@ +# In the future, build the kubernetes cluster from the official docker account +# Also ensure that proper tagging/versioning is done i.e. remove :latest tag and instead use the digest of the container + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: 'gaas-deployment' + labels: +app: gaas-deployment +spec: + selector: +matchLabels: + app: gaas + replicas: 1 + template: +metadata: + name: 'gaas' + labels: +app: gaas +spec: + volumes: +- name: shared-jobs + persistentVolumeClaim: +claimName: shared-jobs-claim +- name: shared-template-catalogs + persistentVolumeClaim: +claimName: shared-template-catalogs-claim +- name: gaas-config + configMap: +name: gaas-config + containers: +- name: gobblin-service + image: will97/gobblin-as-a-service:latest + command: ["./bin/entrypoint.sh"] + args: ["--args", "-DmysqlCredentials.user=$(MYSQL_USERNAME) -DmysqlCredentials.password=$(MYSQL_PASSWORD)"] + env: +- name: MYSQL_USERNAME + valueFrom: +secretKeyRef: + name: mysql-credentials + key: username +- name: MYSQL_PASSWORD + valueFrom: +secretKeyRef: + name: mysql-credentials + key: password + volumeMounts: +- name: shared-jobs + mountPath: /tmp/gobblin-as-service/jobs +- name: shared-template-catalogs Review comment: I'll remove `share-template-catalog`, I was hoping to use it to share default templates among pods but I determined a better way of doing that in a future PR using configMaps `gaas-config` is for the `.conf` file for GaaS, I use this to replace the one defined in the `conf` folder due a change in the configs to support MySQL This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya opened a new pull request #2785: [GOBBLIN-934] bug fix and refactor for handling new job scheduling
jhsenjaliya opened a new pull request #2785: [GOBBLIN-934] bug fix and refactor
for handling new job scheduling
URL: https://github.com/apache/incubator-gobblin/pull/2785
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [x] My PR addresses the following [Gobblin
JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references
them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-934
### Description
- [x] Here are some details about my PR, including screenshots (if
applicable):
scheduling job logic is broken in PathAlterationListenerAdaptorForMonitor
### Tests
- [x] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason: bug fix & refactor
### Commits
- [x] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] bstaudacher commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md for typos
bstaudacher commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md for typos URL: https://github.com/apache/incubator-gobblin/pull/2783#issuecomment-548110947 I would say LGTM and then I can take a look at programatically fixing the rest of the typos that may exist in the docs? Or if you prefer to have this ticket be a catch all we could go that route as well. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2785: [GOBBLIN-934] bug fix and refactor for handling new job scheduling
codecov-io commented on issue #2785: [GOBBLIN-934] bug fix and refactor for handling new job scheduling URL: https://github.com/apache/incubator-gobblin/pull/2785#issuecomment-548118180 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=h1) Report > Merging [#2785](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d9e4f9a39f56c845d82fceb1a0c410655bca09b0?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2785 +/- ## + Coverage 45.32% 45.34% +0.01% Complexity 8858 8858 Files 1894 1894 Lines 7087470855 -19 Branches 7794 7792 -2 + Hits 3212332127 +4 + Misses3578835763 -25 - Partials 2963 2965 +2 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...duler/PathAlterationListenerAdaptorForMonitor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2785/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NjaGVkdWxlci9QYXRoQWx0ZXJhdGlvbkxpc3RlbmVyQWRhcHRvckZvck1vbml0b3IuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2785/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0%> (-7.15%)` | `3% <0%> (ø)` | | | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2785/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `78.12% <0%> (-1.57%)` | `15% <0%> (-1%)` | | | [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2785/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `80.37% <0%> (+0.93%)` | `24% <0%> (ø)` | :arrow_down: | | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2785/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `72.22% <0%> (+2.22%)` | `13% <0%> (ø)` | :arrow_down: | | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2785/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `35.51% <0%> (+2.8%)` | `12% <0%> (+1%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=footer). Last update [d9e4f9a...1904728](https://codecov.io/gh/apache/incubator-gobblin/pull/2785?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340883278
##
File path: bin/gobblin.sh
##
@@ -135,6 +137,7 @@ function print_gobblin_service_usage() {
echo "--job-conf-file Only for mapreduce mode:
configuration file for the job to run"
echo "--helpDisplay this help."
echo "--verbose Display full command used
to start the process."
+echo "--argsAdditional -D args"
Review comment:
@jhsenjaliya I looked through the scripts again, you mean `jvmflags` not
`jvmopts` right? Because the argument parsing seems to only accept `jvmflags`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
Will-Lo commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340892290 ## File path: gobblin-docker/gobblin-service/alpine-gaas-latest/entrypoint.sh ## @@ -15,7 +15,23 @@ # See the License for the specific language governing permissions and # limitations under the License. # + GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)" +JVM_OPT_FLAG="" +# Treat the config env args differently as they need to be combined together in 1 arg +CONFIG_ARGS="" +shopt -s nocasematch +for i in "$@" +do +case "$1" in +--args) Review comment: I realized there was a way to remove all of these hacks, should've looked into more detailed bash... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add
MySQL and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340896160
##
File path: bin/gobblin.sh
##
@@ -135,6 +137,7 @@ function print_gobblin_service_usage() {
echo "--job-conf-file Only for mapreduce mode:
configuration file for the job to run"
echo "--helpDisplay this help."
echo "--verbose Display full command used
to start the process."
+echo "--argsAdditional -D args"
Review comment:
actually thats the typo in the help text `--jvmopts`, and I meant to keep
the jvmopts since that is more intutive, let me fix that since its some more
variable changes and send PR, in meantime, you can assume to pass it as
`jvmopts` Thanks
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya opened a new pull request #2786: fix help text and align it with variable names
jhsenjaliya opened a new pull request #2786: fix help text and align it with
variable names
URL: https://github.com/apache/incubator-gobblin/pull/2786
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [x] My PR addresses the following [Gobblin
JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references
them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-937
### Description
- [x] Here are some details about my PR, including screenshots (if
applicable):
fix help text and align it with variable names
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason: N/A
### Commits
- [x] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add
MySQL and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340896160
##
File path: bin/gobblin.sh
##
@@ -135,6 +137,7 @@ function print_gobblin_service_usage() {
echo "--job-conf-file Only for mapreduce mode:
configuration file for the job to run"
echo "--helpDisplay this help."
echo "--verbose Display full command used
to start the process."
+echo "--argsAdditional -D args"
Review comment:
actually thats the typo in the help text `--jvmopts`, and I meant to keep
the jvmopts since that is more intutive, let me fix that since its some more
variable changes and send PR (pls check #2786 ), so you can assume to pass it
as `jvmopts` Thanks
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2786: [GOBBLIN-937] fix help text and align it with variable names
jhsenjaliya commented on issue #2786: [GOBBLIN-937] fix help text and align it with variable names URL: https://github.com/apache/incubator-gobblin/pull/2786#issuecomment-548150191 @Will-Lo , @sv2000 , can u pls review and merge this? Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#issuecomment-545699470 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=h1) Report > Merging [#2781](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d9e4f9a39f56c845d82fceb1a0c410655bca09b0?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2781 +/- ## - Coverage 45.32% 45.31% -0.01% - Complexity 8858 8859 +1 Files 1894 1894 Lines 7087470874 Branches 7794 7794 - Hits 3212332119 -4 - Misses3578835794 +6 + Partials 2963 2961 -2 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...in/java/org/apache/gobblin/cluster/SingleTask.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFzay5qYXZh) | `73.58% <0%> (-7.55%)` | `9% <0%> (ø)` | | | [...a/org/apache/gobblin/cluster/GobblinHelixTask.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4VGFzay5qYXZh) | `76.08% <0%> (-4.35%)` | `5% <0%> (ø)` | | | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `78.12% <0%> (-1.57%)` | `15% <0%> (-1%)` | | | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `64.81% <0%> (ø)` | `27% <0%> (-1%)` | :arrow_down: | | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `64.44% <0%> (+1.11%)` | `16% <0%> (+1%)` | :arrow_up: | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `100% <0%> (+7.14%)` | `4% <0%> (+1%)` | :arrow_up: | | [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `60% <0%> (+20%)` | `3% <0%> (+1%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=footer). Last update [d9e4f9a...7471330](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2786: [GOBBLIN-937] fix help text and align it with variable names
codecov-io commented on issue #2786: [GOBBLIN-937] fix help text and align it with variable names URL: https://github.com/apache/incubator-gobblin/pull/2786#issuecomment-548156882 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=h1) Report > Merging [#2786](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d9e4f9a39f56c845d82fceb1a0c410655bca09b0?src=pr&el=desc) will **increase** coverage by `<.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2786 +/- ## + Coverage 45.32% 45.33% +<.01% - Complexity 8858 8861 +3 Files 1894 1894 Lines 7087470874 Branches 7794 7794 + Hits 3212332129 +6 + Misses3578835779 -9 - Partials 2963 2966 +3 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2786/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0%> (-7.15%)` | `3% <0%> (ø)` | | | [...main/java/org/apache/gobblin/yarn/YarnService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2786/diff?src=pr&el=tree#diff-Z29iYmxpbi15YXJuL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3lhcm4vWWFyblNlcnZpY2UuamF2YQ==) | `15.68% <0%> (+0.84%)` | `4% <0%> (+1%)` | :arrow_up: | | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2786/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `64.44% <0%> (+1.11%)` | `16% <0%> (+1%)` | :arrow_up: | | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2786/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `35.51% <0%> (+2.8%)` | `12% <0%> (+1%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=footer). Last update [d9e4f9a...34b3971](https://codecov.io/gh/apache/incubator-gobblin/pull/2786?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust opened a new pull request #2787: [GOBBLIN-938] Make job-template resolution available in all JobLaunchers
autumnust opened a new pull request #2787: [GOBBLIN-938] Make job-template
resolution available in all JobLaunchers
URL: https://github.com/apache/incubator-gobblin/pull/2787
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [x] https://issues.apache.org/jira/browse/GOBBLIN-938
### Description
- [x] Here are some details about my PR, including screenshots (if
applicable):
- Getting rid of duplicated `gobblin.job.templates` in GaaS code
- Making the job-template resolution available in `AbstractJobLauncher` so
that all its extensions will be able to resolve the template, including Helix
JobLauncher. Once it is being resolved and added as part of `JobContext`, the
`Source` will be added with this job properties so as passing-down to each
workunits.
### Tests
- [x] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason: Unit tested in `LocalJobLauncher`
### Commits
- [x] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
jhsenjaliya commented on a change in pull request #2781: [GOBBLIN-913] Add
MySQL and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r340896160
##
File path: bin/gobblin.sh
##
@@ -135,6 +137,7 @@ function print_gobblin_service_usage() {
echo "--job-conf-file Only for mapreduce mode:
configuration file for the job to run"
echo "--helpDisplay this help."
echo "--verbose Display full command used
to start the process."
+echo "--argsAdditional -D args"
Review comment:
actually thats the typo in the help text `--jvmopts`, and I meant to keep
the jvmopts since that is more intutive, let me fix that since its some more
variable changes and send PR (pls check #2786 ), until this gets merged, you
can use `--jvmflags` and that will work, after that you can change it to
`--jvmopts` Thanks
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2787: [GOBBLIN-938] Make job-template resolution available in all JobLaunchers
codecov-io commented on issue #2787: [GOBBLIN-938] Make job-template resolution available in all JobLaunchers URL: https://github.com/apache/incubator-gobblin/pull/2787#issuecomment-548224920 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2787?src=pr&el=h1) Report > Merging [#2787](https://codecov.io/gh/apache/incubator-gobblin/pull/2787?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d9e4f9a39f56c845d82fceb1a0c410655bca09b0?src=pr&el=desc) will **increase** coverage by `0.01%`. > The diff coverage is `84.61%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2787?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2787 +/- ## + Coverage 45.32% 45.34% +0.01% - Complexity 8858 8863 +5 Files 1894 1894 Lines 7087470877 +3 Branches 7794 7793 -1 + Hits 3212332138 +15 + Misses3578835779 -9 + Partials 2963 2960 -3 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2787?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...pache/gobblin/cluster/GobblinHelixJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4Sm9iTGF1bmNoZXIuamF2YQ==) | `81.81% <ø> (ø)` | `26 <0> (ø)` | :arrow_down: | | [...org/apache/gobblin/azkaban/AzkabanJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9hemthYmFuL0F6a2FiYW5Kb2JMYXVuY2hlci5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...rg/apache/gobblin/runtime/AbstractJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvQWJzdHJhY3RKb2JMYXVuY2hlci5qYXZh) | `59.13% <100%> (+1.01%)` | `33 <2> (+2)` | :arrow_up: | | [...gobblin/service/modules/spec/JobExecutionPlan.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zcGVjL0pvYkV4ZWN1dGlvblBsYW4uamF2YQ==) | `75.8% <100%> (ø)` | `9 <0> (ø)` | :arrow_down: | | [...odules/template\_catalog/FSFlowTemplateCatalog.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy90ZW1wbGF0ZV9jYXRhbG9nL0ZTRmxvd1RlbXBsYXRlQ2F0YWxvZy5qYXZh) | `58.69% <50%> (ø)` | `8 <0> (ø)` | :arrow_down: | | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.42% <0%> (-1.39%)` | `27% <0%> (-1%)` | | | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `64.44% <0%> (+1.11%)` | `16% <0%> (+1%)` | :arrow_up: | | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `72.22% <0%> (+2.22%)` | `13% <0%> (ø)` | :arrow_down: | | [.../limiter/RedirectAwareRestClientRequestSender.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UtY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9SZWRpcmVjdEF3YXJlUmVzdENsaWVudFJlcXVlc3RTZW5kZXIuamF2YQ==) | `87.2% <0%> (+2.32%)` | `18% <0%> (+1%)` | :arrow_up: | | [...he/gobblin/metrics/reporter/ScheduledReporter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2787/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9yZXBvcnRlci9TY2hlZHVsZWRSZXBvcnRlci5qYXZ
[GitHub] [incubator-gobblin] asfgit closed pull request #2787: [GOBBLIN-938] Make job-template resolution available in all JobLaunchers
asfgit closed pull request #2787: [GOBBLIN-938] Make job-template resolution available in all JobLaunchers URL: https://github.com/apache/incubator-gobblin/pull/2787 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate
jhsenjaliya commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate URL: https://github.com/apache/incubator-gobblin/pull/2751#issuecomment-548469068 @Will-Lo , I am trying to see how can we make this consistent, I think with addition of GOBBLIN_WORK_DIR and GOBBLIN_JOB_CONFIG_DIR envrionment varialble support with ability to override from command line would perfectly work. what do you think? I will make the changes accordingly. Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya opened a new pull request #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
jhsenjaliya opened a new pull request #2788: [GOBBLIN-939] Integrate usage of
env variables in gobblin scripts and configs
URL: https://github.com/apache/incubator-gobblin/pull/2788
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [x] My PR addresses the following [Gobblin
JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references
them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-939
### Description
- [x] Here are some details about my PR, including screenshots (if
applicable):
1. standardize config with ENV variables
2. define default env variables in gobblin-env.sh
### Tests
- [x] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason: N/A
### Commits
- [x] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate
jhsenjaliya commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate URL: https://github.com/apache/incubator-gobblin/pull/2751#issuecomment-548489825 not it looks much better with #2788, pls check it out. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md for typos
autumnust commented on issue #2783: Update Hive-Avro-To-ORC-Converter.md for typos URL: https://github.com/apache/incubator-gobblin/pull/2783#issuecomment-548491406 @bstaudacher Just out of curiosity: Which tool are you planning to address typos given the scale of this code base? I am more than happy to assist on that if you already have a plan. I updated https://issues.apache.org/jira/browse/GOBBLIN-931 to be not specific to Avro-to-ORC. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate
Will-Lo commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate URL: https://github.com/apache/incubator-gobblin/pull/2751#issuecomment-548491660 @jhsenjaliya sure we can go with that, I think the main thing is that the documentation should be updated to reflect the change in workflows that we introduce. I think following an approach with #2788 is good, we just need to test it to ensure we don't introduce regressions This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
Will-Lo commented on a change in pull request #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#discussion_r341287822 ## File path: bin/gobblin-env.sh ## @@ -17,4 +17,6 @@ # limitations under the License. # -# Set Gobblin specific environment variables here. +# export environment variables here to be used in gobblin.sh scripts and gobblin platform configs. +export GOBBLIN_WORK_DIR=$GOBBLIN_HOME/gobblin-work-dir +export GOBBLIN_JOB_CONFIG_DIR=$GOBBLIN_HOME/gobblin-job-config-dir Review comment: I think to keep it more consistent with our old naming conventions we should do `GOBBLIN_WORK_DIR=$GOBBLIN_HOME/work` `GOBBLIN_JOB_CONFIG_DIR=$GOBBLIN_HOME/jobs` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on a change in pull request #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
Will-Lo commented on a change in pull request #2788: [GOBBLIN-939] Integrate
usage of env variables in gobblin scripts and configs
URL: https://github.com/apache/incubator-gobblin/pull/2788#discussion_r341290769
##
File path: bin/gobblin.sh
##
@@ -283,18 +291,18 @@ elif [[ -f ${GOBBLIN_CONF}/log4j.properties ]]; then
LOG4J_OPTS="-Dlog4j.configuration=$LOG4J_FILE_PATH"
fi
+#Create required directories
if [[ ! -d "$GOBBLIN_LOGS" ]]; then
mkdir -p $GOBBLIN_LOGS
fi
-GC_OPTS=''
-if [[ ${ENABLE_GC_LOGS} -eq 1 ]]; then
-GC_OPTS+="-XX:+UseConcMarkSweepGC -XX:+UseParNewGC -XX:+UseCompressedOops "
-GC_OPTS+="-XX:+PrintGCDetails -XX:+PrintGCDateStamps
-XX:+PrintTenuringDistribution "
-GC_OPTS+="-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=$GOBBLIN_LOGS/ "
-GC_OPTS+="-Xloggc:$GOBBLIN_LOGS/gobblin-$GOBBLIN_MODE-gc.log "
+if [[ ! -d "$GOBBLIN_WORK_DIR" ]]; then
+mkdir -p $GOBBLIN_WORK_DIR
fi
+if [[ ! -d "$GOBBLIN_JOB_CONFIG_DIR" ]]; then
+mkdir -p $GOBBLIN_JOB_CONFIG_DIR
+fi
Review comment:
If these are initiated using `gobblin-env.sh`, and called in `gobblin.sh`
and never overwritten before the directories are created, would it be possible
to modify the env variables?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] Will-Lo commented on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
Will-Lo commented on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548500401 I was wondering, would it be better if we were to utilise javaopts, so `-D` flags for these environment variables, rather than continuously modifying the scripts/configs? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on issue #2789: [GOBBLIN-940]Add synchronization on workunit persistency before Helix job launching
autumnust commented on issue #2789: [GOBBLIN-940]Add synchronization on workunit persistency before Helix job launching URL: https://github.com/apache/incubator-gobblin/pull/2789#issuecomment-548508801 @htran1 Can you take a look ? thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust opened a new pull request #2789: [GOBBLIN-940]Add synchronization on workunit persistency before Helix job launching
autumnust opened a new pull request #2789: [GOBBLIN-940]Add synchronization on
workunit persistency before Helix job launching
URL: https://github.com/apache/incubator-gobblin/pull/2789
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [x] https://issues.apache.org/jira/browse/GOBBLIN-940
### Description
- [x] Did some minor refactoring and unit test fixing, the major part is
just adding `waitForTasks ` before Helix job being returned and submitted. This
is causing problem when HDFS being very slow / parallel runner hitting CPU
bounded on working on workunit persistency that Helix task is activated before
WU is showing up.
### Tests
- [x] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason:
### Commits
- [ ] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
codecov-io commented on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548517189 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=h1) Report > Merging [#2788](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #2788 +/- ## === - Coverage 4.15% 4.15% -0.01% + Complexity 746 745 -1 === Files 18941894 Lines 70877 70877 Branches 77937793 === - Hits 29462945 -1 Misses67617 67617 - Partials314 315 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `63.33% <0%> (-1.12%)` | `15% <0%> (-1%)` | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=footer). Last update [9ee4dca...9209128](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548517189 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=h1) Report > Merging [#2788](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **increase** coverage by `41.19%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2788 +/- ## = + Coverage 4.15% 45.35% +41.19% - Complexity 746 8864 +8118 = Files 1894 1894 Lines 7087770877 Branches 7793 7793 = + Hits 294632143+29197 + Misses6761735773-31844 - Partials314 2961 +2647 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `63.33% <0%> (-1.12%)` | `15% <0%> (-1%)` | | | [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0% <0%> (ø)` | `2% <0%> (+2%)` | :arrow_up: | | [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.95% <0%> (+0.95%)` | `1% <0%> (+1%)` | :arrow_up: | | [...ain/java/org/apache/gobblin/runtime/TaskState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlLmphdmE=) | `81.97% <0%> (+1.16%)` | `32% <0%> (ø)` | :arrow_down: | | [...pache/gobblin/runtime/GobblinMultiTaskAttempt.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvR29iYmxpbk11bHRpVGFza0F0dGVtcHQuamF2YQ==) | `56.3% <0%> (+1.35%)` | `27% <0%> (+2%)` | :arrow_up: | | [...ava/org/apache/gobblin/runtime/MultiConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvTXVsdGlDb252ZXJ0ZXIuamF2YQ==) | `83.6% <0%> (+1.63%)` | `9% <0%> (+1%)` | :arrow_up: | | [...rg/apache/gobblin/runtime/FsDatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvRnNEYXRhc2V0U3RhdGVTdG9yZS5qYXZh) | `73.8% <0%> (+1.78%)` | `35% <0%> (+1%)` | :arrow_up: | | [...a/org/apache/gobblin/cluster/SingleTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFza1J1bm5lci5qYXZh) | `1.85% <0%> (+1.85%)` | `1% <0%> (+1%)` | :arrow_up: | | [.../java/org/apache/gobblin/runtime/TaskExecutor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza0V4ZWN1dG9yLmphdmE=) | `45.05% <0%> (+2.19%)` | `9% <0%> (+1%)` | :arrow_up: | | [...apache/gobblin/source/jdbc/SqlServerExtractor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9qZGJjL1NxbFNlcnZlckV4dHJhY3Rvci5qYXZh) | `4.41% <0%> (+4.41%)` | `3% <0%> (+3%)` | :arrow_up: | | ... and [1088 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](http
[GitHub] [incubator-gobblin] codecov-io commented on issue #2789: [GOBBLIN-940]Add synchronization on workunit persistency before Helix job launching
codecov-io commented on issue #2789: [GOBBLIN-940]Add synchronization on workunit persistency before Helix job launching URL: https://github.com/apache/incubator-gobblin/pull/2789#issuecomment-548527186 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2789?src=pr&el=h1) Report > Merging [#2789](https://codecov.io/gh/apache/incubator-gobblin/pull/2789?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **increase** coverage by `41.19%`. > The diff coverage is `100%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2789?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2789 +/- ## = + Coverage 4.15% 45.35% +41.19% - Complexity 746 8867 +8121 = Files 1894 1894 Lines 7087770879+2 Branches 7793 7793 = + Hits 294632145+29199 + Misses6761735772-31845 - Partials314 2962 +2648 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2789?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...pache/gobblin/cluster/GobblinHelixJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4Sm9iTGF1bmNoZXIuamF2YQ==) | `82% <100%> (+82%)` | `27 <0> (+27)` | :arrow_up: | | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `63.33% <0%> (-1.12%)` | `15% <0%> (-1%)` | | | [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0% <0%> (ø)` | `2% <0%> (+2%)` | :arrow_up: | | [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.95% <0%> (+0.95%)` | `1% <0%> (+1%)` | :arrow_up: | | [...ain/java/org/apache/gobblin/runtime/TaskState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlLmphdmE=) | `81.97% <0%> (+1.16%)` | `32% <0%> (ø)` | :arrow_down: | | [...pache/gobblin/runtime/GobblinMultiTaskAttempt.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvR29iYmxpbk11bHRpVGFza0F0dGVtcHQuamF2YQ==) | `56.3% <0%> (+1.35%)` | `27% <0%> (+2%)` | :arrow_up: | | [...ava/org/apache/gobblin/runtime/MultiConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvTXVsdGlDb252ZXJ0ZXIuamF2YQ==) | `83.6% <0%> (+1.63%)` | `9% <0%> (+1%)` | :arrow_up: | | [...rg/apache/gobblin/runtime/FsDatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvRnNEYXRhc2V0U3RhdGVTdG9yZS5qYXZh) | `73.8% <0%> (+1.78%)` | `35% <0%> (+1%)` | :arrow_up: | | [...a/org/apache/gobblin/cluster/SingleTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFza1J1bm5lci5qYXZh) | `1.85% <0%> (+1.85%)` | `1% <0%> (+1%)` | :arrow_up: | | [.../java/org/apache/gobblin/runtime/TaskExecutor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza0V4ZWN1dG9yLmphdmE=) | `45.05% <0%> (+2.19%)` | `9% <0%> (+1%)` | :arrow_up: | | ... and [1089 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2789/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/
[GitHub] [incubator-gobblin] ZihanLi58 opened a new pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 opened a new pull request #2790: [GOBBLIN-941] Enhance DDL to add
column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [ ] My PR addresses the following [Gobblin
JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references
them in the PR title. For example, "[GOBBLIN-941] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-941
### Description
- [ ] Here are some details about my PR, including screenshots (if
applicable):
Enhance DDL to add column and column.types with case-preserving schema which
would enforce avro2orc output preserving correct casing
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason:
Unit test and test the avroToOrc pipeline to make sure the output preserving
correct casing
### Commits
- [ ] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] asfgit closed pull request #2780: [GOBBLIN-924]Get rid of orc.schema.literal in ORC-ingestion and registration
asfgit closed pull request #2780: [GOBBLIN-924]Get rid of orc.schema.literal in ORC-ingestion and registration URL: https://github.com/apache/incubator-gobblin/pull/2780 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate
jhsenjaliya commented on issue #2751: [GOBBLIN-895] Fixes Gobblin Standalone configs and scripts so that user guide is accurate URL: https://github.com/apache/incubator-gobblin/pull/2751#issuecomment-548558371 Sure, let me updates the docs accordingly. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 closed pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 closed pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 opened a new pull request #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 opened a new pull request #2791: [GOBBLIN-941] Enhance DDL to add
column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2791
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I
have checked off all the steps below!
### JIRA
- [ ] My PR addresses the following [Gobblin
JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references
them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-941
### Description
- [ ] Here are some details about my PR, including screenshots (if
applicable):
Enhance DDL to add column and column.types with case-preserving schema which
would enforce avro2orc output preserving correct casing
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason:
Unit test and test the avroToOrc pipeline to make sure the output preserving
correct casing
### Commits
- [ ] My commits all reference JIRA issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
codecov-io commented on issue #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#issuecomment-548564313 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=h1) Report > Merging [#2790](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #2790 +/- ## === - Coverage 4.15% 4.15% -0.01% Complexity 746 746 === Files 18941894 Lines 70877 70911 +34 Branches 77937799 +6 === Hits 29462946 - Misses67617 67651 +34 Partials314 314 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...g/apache/gobblin/hive/orc/HiveOrcSerDeManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1oaXZlLXJlZ2lzdHJhdGlvbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9oaXZlL29yYy9IaXZlT3JjU2VyRGVNYW5hZ2VyLmphdmE=) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ion/hive/converter/AbstractAvroToOrcConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9jb252ZXJ0ZXIvQWJzdHJhY3RBdnJvVG9PcmNDb252ZXJ0ZXIuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...nversion/hive/query/HiveAvroORCQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9xdWVyeS9IaXZlQXZyb09SQ1F1ZXJ5R2VuZXJhdG9yLmphdmE=) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...izer/HiveMaterializerFromEntityQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9tYXRlcmlhbGl6ZXIvSGl2ZU1hdGVyaWFsaXplckZyb21FbnRpdHlRdWVyeUdlbmVyYXRvci5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...ement/conversion/hive/task/HiveConverterUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS90YXNrL0hpdmVDb252ZXJ0ZXJVdGlscy5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=footer). Last update [9ee4dca...50639f2](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
codecov-io edited a comment on issue #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#issuecomment-548564313 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=h1) Report > Merging [#2790](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **increase** coverage by `41.17%`. > The diff coverage is `28.57%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2790 +/- ## = + Coverage 4.15% 45.33% +41.17% - Complexity 746 8864 +8118 = Files 1894 1894 Lines 7087770911 +34 Branches 7793 7799+6 = + Hits 294632148+29202 + Misses6761735797-31820 - Partials314 2966 +2652 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2790?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...g/apache/gobblin/hive/orc/HiveOrcSerDeManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1oaXZlLXJlZ2lzdHJhdGlvbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9oaXZlL29yYy9IaXZlT3JjU2VyRGVNYW5hZ2VyLmphdmE=) | `63.95% <0%> (+63.95%)` | `12 <0> (+12)` | :arrow_up: | | [...ement/conversion/hive/task/HiveConverterUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS90YXNrL0hpdmVDb252ZXJ0ZXJVdGlscy5qYXZh) | `51.59% <0%> (+51.59%)` | `23 <0> (+23)` | :arrow_up: | | [...ion/hive/converter/AbstractAvroToOrcConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9jb252ZXJ0ZXIvQWJzdHJhY3RBdnJvVG9PcmNDb252ZXJ0ZXIuamF2YQ==) | `58.36% <100%> (+58.36%)` | `15 <0> (+15)` | :arrow_up: | | [...izer/HiveMaterializerFromEntityQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9tYXRlcmlhbGl6ZXIvSGl2ZU1hdGVyaWFsaXplckZyb21FbnRpdHlRdWVyeUdlbmVyYXRvci5qYXZh) | `89.61% <16.66%> (+89.61%)` | `6 <0> (+6)` | :arrow_up: | | [...nversion/hive/query/HiveAvroORCQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9xdWVyeS9IaXZlQXZyb09SQ1F1ZXJ5R2VuZXJhdG9yLmphdmE=) | `67.2% <76.92%> (+67.2%)` | `84 <1> (+84)` | :arrow_up: | | [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0% <0%> (ø)` | `2% <0%> (+2%)` | :arrow_up: | | [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.95% <0%> (+0.95%)` | `1% <0%> (+1%)` | :arrow_up: | | [...ain/java/org/apache/gobblin/runtime/TaskState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlLmphdmE=) | `81.97% <0%> (+1.16%)` | `32% <0%> (ø)` | :arrow_down: | | [...pache/gobblin/runtime/GobblinMultiTaskAttempt.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvR29iYmxpbk11bHRpVGFza0F0dGVtcHQuamF2YQ==) | `56.3% <0%> (+1.35%)` | `27% <0%> (+2%)` | :arrow_up: | | [...ava/org/apache/gobblin/runtime/MultiConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2790/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2Ji
[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
jhsenjaliya commented on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548566219 > I was wondering, would it be better if we were to utilise javaopts, so `-D` flags for these environment variables, rather than continuously modifying the scripts/configs? u mean like javaopts in bash ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
jhsenjaliya commented on a change in pull request #2788: [GOBBLIN-939]
Integrate usage of env variables in gobblin scripts and configs
URL: https://github.com/apache/incubator-gobblin/pull/2788#discussion_r341362631
##
File path: bin/gobblin.sh
##
@@ -283,18 +291,18 @@ elif [[ -f ${GOBBLIN_CONF}/log4j.properties ]]; then
LOG4J_OPTS="-Dlog4j.configuration=$LOG4J_FILE_PATH"
fi
+#Create required directories
if [[ ! -d "$GOBBLIN_LOGS" ]]; then
mkdir -p $GOBBLIN_LOGS
fi
-GC_OPTS=''
-if [[ ${ENABLE_GC_LOGS} -eq 1 ]]; then
-GC_OPTS+="-XX:+UseConcMarkSweepGC -XX:+UseParNewGC -XX:+UseCompressedOops "
-GC_OPTS+="-XX:+PrintGCDetails -XX:+PrintGCDateStamps
-XX:+PrintTenuringDistribution "
-GC_OPTS+="-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=$GOBBLIN_LOGS/ "
-GC_OPTS+="-Xloggc:$GOBBLIN_LOGS/gobblin-$GOBBLIN_MODE-gc.log "
+if [[ ! -d "$GOBBLIN_WORK_DIR" ]]; then
+mkdir -p $GOBBLIN_WORK_DIR
fi
+if [[ ! -d "$GOBBLIN_JOB_CONFIG_DIR" ]]; then
+mkdir -p $GOBBLIN_JOB_CONFIG_DIR
+fi
Review comment:
do u mean if the change in env variables in `gobblin-env.sh` will get
reflected and get dir created accordingly? yes. since `gobblin.sh` includes the
`gobblin-env.sh`. I have added option to override it from user input as well, i
think it will be useful to maintain any backward compatibility.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341357961 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/task/HiveConverterUtils.java ## @@ -151,6 +154,30 @@ public static String generateCreateDuplicateTableDDL( dbName, tblName, inputDbName, inputTblName, tblLocation); } + public static String generateAlterSchemaDML( + String tableName, + Optional optionalDbName, Review comment: I am not sure why dbName is optional This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341358264 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/task/HiveConverterUtils.java ## @@ -151,6 +154,30 @@ public static String generateCreateDuplicateTableDDL( dbName, tblName, inputDbName, inputTblName, tblLocation); } + public static String generateAlterSchemaDML( Review comment: Shall we use `generateAlterSerDePropsDML`? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341363858 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/materializer/HiveMaterializerFromEntityQueryGenerator.java ## @@ -118,11 +119,19 @@ public QueryBasedHivePublishEntity generatePublishQueries() throws DataConversio Map publishDirectories = publishEntity.getPublishDirectories(); List cleanupQueries = publishEntity.getCleanupQueries(); List cleanupDirectories = publishEntity.getCleanupDirectories(); +Optional avroSchema = Optional.absent(); Review comment: I might be wrong but is this method being called through the conversion jobs? By briefly looking at the code base, this `generatePublishQueries` method is only called in `org.apache.gobblin.data.management.conversion.hive.materializer.HiveMaterializer#generatePublishQueries` which is not related to Avro2ORC, just want to confirm. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341356298 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/converter/AbstractAvroToOrcConverter.java ## @@ -85,6 +86,7 @@ * Subdirectory within destination ORC table directory to publish data */ private static final String PUBLISHED_TABLE_SUBDIRECTORY = "final"; + public static final String OUTPUT_AVRO_SCHEMA_KEY = "output.avro.schema"; Review comment: Does it need to be public static ? Limit access modifiers so that it won't be accidentally touched by irrelevant constructs in the future. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance
DDL to add column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341361806
##
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/query/HiveAvroORCQueryGenerator.java
##
@@ -202,7 +204,21 @@ public static String generateCreateTableDDL(Schema schema,
//.. use columns from destination schema
if (isEvolutionEnabled || !destinationTableMeta.isPresent()) {
log.info("Generating DDL using source schema");
+ System.out.println("Generating DDL using source schema");
ddl.append(generateAvroToHiveColumnMapping(schema,
Optional.of(hiveColumns), true, dbName + "." + tblName));
+ try {
Review comment:
Why adding table properties only happens in this branch ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance
DDL to add column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341357486
##
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/query/HiveAvroORCQueryGenerator.java
##
@@ -202,7 +204,21 @@ public static String generateCreateTableDDL(Schema schema,
//.. use columns from destination schema
if (isEvolutionEnabled || !destinationTableMeta.isPresent()) {
log.info("Generating DDL using source schema");
+ System.out.println("Generating DDL using source schema");
Review comment:
Do you still need it?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance
DDL to add column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341358581
##
File path:
gobblin-hive-registration/src/main/java/org/apache/gobblin/hive/orc/HiveOrcSerDeManager.java
##
@@ -61,8 +61,6 @@
*/
@Slf4j
public class HiveOrcSerDeManager extends HiveSerDeManager {
- // Schema is in the format of TypeDescriptor
- public static final String SCHEMA_LITERAL = "orc.schema.literal";
Review comment:
Do a rebase and push with force option to avoid old commits
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341366268 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/converter/AbstractAvroToOrcConverter.java ## @@ -85,6 +86,7 @@ * Subdirectory within destination ORC table directory to publish data */ private static final String PUBLISHED_TABLE_SUBDIRECTORY = "final"; + public static final String OUTPUT_AVRO_SCHEMA_KEY = "output.avro.schema"; Review comment: This one will be access in HiveMaterializerFromEntityQueryGenerator when it try to get the schema. So I set it to be public static. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341366880 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/task/HiveConverterUtils.java ## @@ -151,6 +154,30 @@ public static String generateCreateDuplicateTableDDL( dbName, tblName, inputDbName, inputTblName, tblLocation); } + public static String generateAlterSchemaDML( + String tableName, + Optional optionalDbName, Review comment: Since I try to make it consistent with method generateCreateDuplicateTableDDL. In which the dbName is optional. I guess it's for testing. But not so sure about that This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341370332 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/materializer/HiveMaterializerFromEntityQueryGenerator.java ## @@ -118,11 +119,19 @@ public QueryBasedHivePublishEntity generatePublishQueries() throws DataConversio Map publishDirectories = publishEntity.getPublishDirectories(); List cleanupQueries = publishEntity.getCleanupQueries(); List cleanupDirectories = publishEntity.getCleanupDirectories(); +Optional avroSchema = Optional.absent(); Review comment: This is not called by Avro2Roc. I add this just to make sure it's consistent with that. But I can remove this one if needed This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance
DDL to add column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341369711
##
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/query/HiveAvroORCQueryGenerator.java
##
@@ -202,7 +204,21 @@ public static String generateCreateTableDDL(Schema schema,
//.. use columns from destination schema
if (isEvolutionEnabled || !destinationTableMeta.isPresent()) {
log.info("Generating DDL using source schema");
+ System.out.println("Generating DDL using source schema");
ddl.append(generateAvroToHiveColumnMapping(schema,
Optional.of(hiveColumns), true, dbName + "." + tblName));
+ try {
Review comment:
I want to make sure only when schema evolution is enabled or there is no
existing table, we can set the columns. Because that will overwrite the schema.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io commented on issue #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
codecov-io commented on issue #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2791#issuecomment-548577059 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=h1) Report > Merging [#2791](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/f60409ef0b6768bf46ddd137333d8d56981798fc?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `36.36%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2791 +/- ## - Coverage 45.34% 45.33% -0.01% Complexity 8863 8863 Files 1894 1894 Lines 7087970911 +32 Branches 7795 7799 +4 + Hits 3213732148 +11 - Misses3577835799 +21 Partials 2964 2964 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ement/conversion/hive/task/HiveConverterUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS90YXNrL0hpdmVDb252ZXJ0ZXJVdGlscy5qYXZh) | `51.59% <0%> (-4.66%)` | `23 <0> (ø)` | | | [...ion/hive/converter/AbstractAvroToOrcConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9jb252ZXJ0ZXIvQWJzdHJhY3RBdnJvVG9PcmNDb252ZXJ0ZXIuamF2YQ==) | `58.36% <100%> (+0.17%)` | `15 <0> (ø)` | :arrow_down: | | [...izer/HiveMaterializerFromEntityQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9tYXRlcmlhbGl6ZXIvSGl2ZU1hdGVyaWFsaXplckZyb21FbnRpdHlRdWVyeUdlbmVyYXRvci5qYXZh) | `89.61% <16.66%> (-6.17%)` | `6 <0> (ø)` | | | [...nversion/hive/query/HiveAvroORCQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9xdWVyeS9IaXZlQXZyb09SQ1F1ZXJ5R2VuZXJhdG9yLmphdmE=) | `67.2% <76.92%> (+0.19%)` | `84 <1> (+1)` | :arrow_up: | | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `70% <0%> (-2.23%)` | `13% <0%> (ø)` | | | [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `80.37% <0%> (+0.93%)` | `24% <0%> (ø)` | :arrow_down: | | [...lin/elasticsearch/writer/FutureCallbackHolder.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3dyaXRlci9GdXR1cmVDYWxsYmFja0hvbGRlci5qYXZh) | `62.85% <0%> (+1.42%)` | `4% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=footer). Last update [f60409e...42a3dbe](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548517189 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=h1) Report > Merging [#2788](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **increase** coverage by `41.18%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2788 +/- ## = + Coverage 4.15% 45.33% +41.18% - Complexity 746 8863 +8117 = Files 1894 1894 Lines 7087770879+2 Branches 7793 7795+2 = + Hits 294632136+29190 + Misses6761735781-31836 - Partials314 2962 +2648 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0% <0%> (ø)` | `2% <0%> (+2%)` | :arrow_up: | | [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.95% <0%> (+0.95%)` | `1% <0%> (+1%)` | :arrow_up: | | [...ain/java/org/apache/gobblin/runtime/TaskState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlLmphdmE=) | `81.97% <0%> (+1.16%)` | `32% <0%> (ø)` | :arrow_down: | | [...pache/gobblin/runtime/GobblinMultiTaskAttempt.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvR29iYmxpbk11bHRpVGFza0F0dGVtcHQuamF2YQ==) | `56.3% <0%> (+1.35%)` | `27% <0%> (+2%)` | :arrow_up: | | [...ava/org/apache/gobblin/runtime/MultiConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvTXVsdGlDb252ZXJ0ZXIuamF2YQ==) | `83.6% <0%> (+1.63%)` | `9% <0%> (+1%)` | :arrow_up: | | [...rg/apache/gobblin/runtime/FsDatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvRnNEYXRhc2V0U3RhdGVTdG9yZS5qYXZh) | `73.8% <0%> (+1.78%)` | `35% <0%> (+1%)` | :arrow_up: | | [...a/org/apache/gobblin/cluster/SingleTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFza1J1bm5lci5qYXZh) | `1.85% <0%> (+1.85%)` | `1% <0%> (+1%)` | :arrow_up: | | [.../java/org/apache/gobblin/runtime/TaskExecutor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza0V4ZWN1dG9yLmphdmE=) | `45.05% <0%> (+2.19%)` | `9% <0%> (+1%)` | :arrow_up: | | [...apache/gobblin/source/jdbc/SqlServerExtractor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9qZGJjL1NxbFNlcnZlckV4dHJhY3Rvci5qYXZh) | `4.41% <0%> (+4.41%)` | `3% <0%> (+3%)` | :arrow_up: | | [...rg/apache/gobblin/runtime/AbstractJobLauncher.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvQWJzdHJhY3RKb2JMYXVuY2hlci5qYXZh) | `59.13% <0%> (+5.1%)` | `33% <0%> (+6%)` | :arrow_up: | | ... and [1087 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https:
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
codecov-io edited a comment on issue #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2791#issuecomment-548577059 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=h1) Report > Merging [#2791](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/f60409ef0b6768bf46ddd137333d8d56981798fc?src=pr&el=desc) will **decrease** coverage by `<.01%`. > The diff coverage is `34.37%`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2791 +/- ## - Coverage 45.34% 45.33% -0.01% + Complexity 8863 8862 -1 Files 1894 1894 Lines 7087970910 +31 Branches 7795 7799 +4 + Hits 3213732146 +9 - Misses3577835798 +20 - Partials 2964 2966 +2 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ement/conversion/hive/task/HiveConverterUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS90YXNrL0hpdmVDb252ZXJ0ZXJVdGlscy5qYXZh) | `51.59% <0%> (-4.66%)` | `23 <0> (ø)` | | | [...ion/hive/converter/AbstractAvroToOrcConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9jb252ZXJ0ZXIvQWJzdHJhY3RBdnJvVG9PcmNDb252ZXJ0ZXIuamF2YQ==) | `58.36% <100%> (+0.17%)` | `15 <0> (ø)` | :arrow_down: | | [...izer/HiveMaterializerFromEntityQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9tYXRlcmlhbGl6ZXIvSGl2ZU1hdGVyaWFsaXplckZyb21FbnRpdHlRdWVyeUdlbmVyYXRvci5qYXZh) | `89.61% <16.66%> (-6.17%)` | `6 <0> (ø)` | | | [...nversion/hive/query/HiveAvroORCQueryGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9xdWVyeS9IaXZlQXZyb09SQ1F1ZXJ5R2VuZXJhdG9yLmphdmE=) | `67.13% <75%> (+0.12%)` | `84 <1> (+1)` | :arrow_up: | | [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `40% <0%> (-20%)` | `2% <0%> (-1%)` | | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `92.85% <0%> (-7.15%)` | `3% <0%> (-1%)` | | | [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2791/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `80.37% <0%> (+0.93%)` | `24% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=footer). Last update [f60409e...42a3dbe](https://codecov.io/gh/apache/incubator-gobblin/pull/2791?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regard
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs
codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548517189 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=h1) Report > Merging [#2788](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/9ee4dcaf66257b6e2926cf1470b16b912cd343ff?src=pr&el=desc) will **increase** coverage by `41.18%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2788 +/- ## = + Coverage 4.15% 45.34% +41.18% - Complexity 746 8863 +8117 = Files 1894 1894 Lines 7087770879+2 Branches 7793 7795+2 = + Hits 294632139+29193 + Misses6761735778-31839 - Partials314 2962 +2648 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...e/gobblin/runtime/locks/ZookeeperBasedJobLock.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbG9ja3MvWm9va2VlcGVyQmFzZWRKb2JMb2NrLmphdmE=) | `63.33% <0%> (-1.12%)` | `15% <0%> (-1%)` | | | [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0% <0%> (ø)` | `2% <0%> (+2%)` | :arrow_up: | | [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.95% <0%> (+0.95%)` | `1% <0%> (+1%)` | :arrow_up: | | [...ain/java/org/apache/gobblin/runtime/TaskState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlLmphdmE=) | `81.97% <0%> (+1.16%)` | `32% <0%> (ø)` | :arrow_down: | | [...pache/gobblin/runtime/GobblinMultiTaskAttempt.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvR29iYmxpbk11bHRpVGFza0F0dGVtcHQuamF2YQ==) | `56.3% <0%> (+1.35%)` | `27% <0%> (+2%)` | :arrow_up: | | [...ava/org/apache/gobblin/runtime/MultiConverter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvTXVsdGlDb252ZXJ0ZXIuamF2YQ==) | `83.6% <0%> (+1.63%)` | `9% <0%> (+1%)` | :arrow_up: | | [...rg/apache/gobblin/runtime/FsDatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvRnNEYXRhc2V0U3RhdGVTdG9yZS5qYXZh) | `73.8% <0%> (+1.78%)` | `35% <0%> (+1%)` | :arrow_up: | | [...a/org/apache/gobblin/cluster/SingleTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFza1J1bm5lci5qYXZh) | `1.85% <0%> (+1.85%)` | `1% <0%> (+1%)` | :arrow_up: | | [.../java/org/apache/gobblin/runtime/TaskExecutor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza0V4ZWN1dG9yLmphdmE=) | `45.05% <0%> (+2.19%)` | `9% <0%> (+1%)` | :arrow_up: | | [...apache/gobblin/source/jdbc/SqlServerExtractor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9qZGJjL1NxbFNlcnZlckV4dHJhY3Rvci5qYXZh) | `4.41% <0%> (+4.41%)` | `3% <0%> (+3%)` | :arrow_up: | | ... and [1088 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](http
[GitHub] [incubator-gobblin] ZihanLi58 commented on issue #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 commented on issue #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2791#issuecomment-548599519 @autumnust Here is the new PR, Thx. ^^ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] asfgit closed pull request #2786: [GOBBLIN-937] fix help text and align it with variable names
asfgit closed pull request #2786: [GOBBLIN-937] fix help text and align it with variable names URL: https://github.com/apache/incubator-gobblin/pull/2786 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341414676 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/materializer/HiveMaterializerFromEntityQueryGenerator.java ## @@ -118,11 +119,19 @@ public QueryBasedHivePublishEntity generatePublishQueries() throws DataConversio Map publishDirectories = publishEntity.getPublishDirectories(); List cleanupQueries = publishEntity.getCleanupQueries(); List cleanupDirectories = publishEntity.getCleanupDirectories(); +Optional avroSchema = Optional.absent(); Review comment: You call. I am OK with both. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341414676 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/materializer/HiveMaterializerFromEntityQueryGenerator.java ## @@ -118,11 +119,19 @@ public QueryBasedHivePublishEntity generatePublishQueries() throws DataConversio Map publishDirectories = publishEntity.getPublishDirectories(); List cleanupQueries = publishEntity.getCleanupQueries(); List cleanupDirectories = publishEntity.getCleanupDirectories(); +Optional avroSchema = Optional.absent(); Review comment: Your call. I am OK with both. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance
DDL to add column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341415129
##
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/query/HiveAvroORCQueryGenerator.java
##
@@ -202,7 +204,21 @@ public static String generateCreateTableDDL(Schema schema,
//.. use columns from destination schema
if (isEvolutionEnabled || !destinationTableMeta.isPresent()) {
log.info("Generating DDL using source schema");
+ System.out.println("Generating DDL using source schema");
ddl.append(generateAvroToHiveColumnMapping(schema,
Optional.of(hiveColumns), true, dbName + "." + tblName));
+ try {
Review comment:
Fair enough. Worth to check if the production have schema evolution enabled.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
autumnust commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341415701 ## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/task/HiveConverterUtils.java ## @@ -151,6 +154,30 @@ public static String generateCreateDuplicateTableDDL( dbName, tblName, inputDbName, inputTblName, tblLocation); } + public static String generateAlterSchemaDML( + String tableName, + Optional optionalDbName, Review comment: +1 for consistency. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
ZihanLi58 commented on a change in pull request #2790: [GOBBLIN-941] Enhance
DDL to add column and column.types with case-preserving schema
URL: https://github.com/apache/incubator-gobblin/pull/2790#discussion_r341419272
##
File path:
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/conversion/hive/query/HiveAvroORCQueryGenerator.java
##
@@ -202,7 +204,21 @@ public static String generateCreateTableDDL(Schema schema,
//.. use columns from destination schema
if (isEvolutionEnabled || !destinationTableMeta.isPresent()) {
log.info("Generating DDL using source schema");
+ System.out.println("Generating DDL using source schema");
ddl.append(generateAvroToHiveColumnMapping(schema,
Optional.of(hiveColumns), true, dbName + "." + tblName));
+ try {
Review comment:
Yes, at least it's enabled in scoreevent. BTW, can you look at the new pr
for this commit? I have sent you the email. Thank you!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r341681213 ## File path: gobblin-kubernetes/gobblin-service/dev-cluster/mysql-deployment.yaml ## @@ -0,0 +1,55 @@ +apiVersion: v1 Review comment: How do we organize the content between `fs` based and `mysql` based? Using the same parent folder `dev-cluster` is confusing. Currently, we're using `kustomization` file to share and organize k8s application configs. I guess we'll have a mysql folder with its own `kustomization` file? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
zxcware commented on a change in pull request #2781: [GOBBLIN-913] Add MySQL
and configurations to cluster
URL: https://github.com/apache/incubator-gobblin/pull/2781#discussion_r341678716
##
File path: gobblin-kubernetes/gobblin-service/dev-cluster/gaas-application.conf
##
@@ -0,0 +1,73 @@
+#
+# 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.
+#
+
+# Sample configuration properties for the Gobblin Service
+
+# Topology Catalog and Store
+gobblin.service.work.dir=/tmp/gobblin-as-service
+
+# TopologySpec Factory
+topologySpec.store.dir=${gobblin.service.work.dir}/topologySpecStore
+topologySpecFactory.topologyNames=localGobblinCluster
+topologySpecFactory.localGobblinCluster.description="StandaloneClusterTopology"
+topologySpecFactory.localGobblinCluster.version="1"
+topologySpecFactory.localGobblinCluster.uri="gobblinCluster"
+topologySpecFactory.localGobblinCluster.specExecutorInstance.class="org.apache.gobblin.runtime.spec_executorInstance.LocalFsSpecExecutor"
+topologySpecFactory.localGobblinCluster.specExecInstance.capabilities="source:dest"
+topologySpecFactory.localGobblinCluster.gobblin.cluster.localSpecProducer.dir=${gobblin.service.work.dir}/jobs
+
+# Flow Catalog and Store
+flowSpec.store.dir=${gobblin.service.work.dir}/flowSpecStore
+
+# Template Catalog
+gobblin.service.templateCatalogs.fullyQualifiedPath="file://"
+
+# JobStatusMonitor
+gobblin.service.jobStatusMonitor.enabled=false
+
+# FsJobStatusRetriever
+fsJobStatusRetriever.state.store.dir=${gobblin.service.work.dir}/state-store
+
+# DagManager
+gobblin.service.dagManager.enabled=true
+gobblin.service.dagManager.jobStatusRetriever.class="org.apache.gobblin.service.monitoring.FsJobStatusRetriever"
+gobblin.service.dagManager.dagStateStoreClass="org.apache.gobblin.service.modules.orchestration.FSDagStateStore"
+gobblin.service.dagManager.dagStateStoreDir=${gobblin.service.work.dir}/dagStateStoreDir
+
+# RestLI
+gobblin.service.port=6956
+
+# MySQL State Store
Review comment:
if `dev` is built around file systems, do we need `mysql` environment here?
A user can pick either `dev` suite or `mysql` suite.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster
codecov-io edited a comment on issue #2781: [GOBBLIN-913] Add MySQL and configurations to cluster URL: https://github.com/apache/incubator-gobblin/pull/2781#issuecomment-545699470 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=h1) Report > Merging [#2781](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/60e3ab56a6a5bd1fd7c7c31586d6739581fe354f?src=pr&el=desc) will **increase** coverage by `<.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2781 +/- ## + Coverage 45.34% 45.34% +<.01% + Complexity 8864 8861 -3 Files 1894 1894 Lines 7087970879 Branches 7795 7795 + Hits 3214132143 +2 + Misses3577635770 -6 - Partials 2962 2966 +4 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `40% <0%> (-20%)` | `2% <0%> (-1%)` | | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `92.85% <0%> (-7.15%)` | `3% <0%> (-1%)` | | | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `78.12% <0%> (-1.57%)` | `15% <0%> (-1%)` | | | [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `64.81% <0%> (-0.47%)` | `28% <0%> (ø)` | | | [...main/java/org/apache/gobblin/yarn/YarnService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi15YXJuL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3lhcm4vWWFyblNlcnZpY2UuamF2YQ==) | `15.68% <0%> (+0.84%)` | `4% <0%> (+1%)` | :arrow_up: | | [...lin/elasticsearch/writer/FutureCallbackHolder.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3dyaXRlci9GdXR1cmVDYWxsYmFja0hvbGRlci5qYXZh) | `62.85% <0%> (+1.42%)` | `4% <0%> (ø)` | :arrow_down: | | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2781/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `72.22% <0%> (+2.22%)` | `13% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=footer). Last update [60e3ab5...348aedb](https://codecov.io/gh/apache/incubator-gobblin/pull/2781?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] htran1 commented on a change in pull request #2789: [GOBBLIN-940]Add synchronization on workunit persistency before Helix job launching
htran1 commented on a change in pull request #2789: [GOBBLIN-940]Add
synchronization on workunit persistency before Helix job launching
URL: https://github.com/apache/incubator-gobblin/pull/2789#discussion_r341686549
##
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixJobLauncher.java
##
@@ -295,23 +295,34 @@ protected void executeCancellation() {
SerializationUtils.serializeState(this.fs, jobStateFilePath,
this.jobContext.getJobState());
}
- LOGGER.debug("GobblinHelixJobLauncher.createJob: jobStateFilePath {},
jobState {} jobProperties {}",
+ // Block on all persistency of workunits finished
+ // It is necessary when underlying storage being and Helix activate
task-execution before the workunit being persisted.
Review comment:
`storage being and`? Seems like something is missing here in this comment.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and standardize configs
codecov-io edited a comment on issue #2788: [GOBBLIN-939] Integrate usage of env variables in gobblin scripts and standardize configs URL: https://github.com/apache/incubator-gobblin/pull/2788#issuecomment-548517189 # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=h1) Report > Merging [#2788](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/60e3ab56a6a5bd1fd7c7c31586d6739581fe354f?src=pr&el=desc) will **decrease** coverage by `0.01%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#2788 +/- ## - Coverage 45.34% 45.32% -0.02% + Complexity 8864 8861 -3 Files 1894 1894 Lines 7087970879 Branches 7795 7795 - Hits 3214132129 -12 - Misses3577635787 +11 - Partials 2962 2963 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `40% <0%> (-20%)` | `2% <0%> (-1%)` | | | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0%> (-14.29%)` | `3% <0%> (-1%)` | | | [...in/java/org/apache/gobblin/cluster/SingleTask.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvU2luZ2xlVGFzay5qYXZh) | `73.58% <0%> (-7.55%)` | `9% <0%> (ø)` | | | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `32.71% <0%> (-6.55%)` | `11% <0%> (-2%)` | | | [...a/org/apache/gobblin/cluster/GobblinHelixTask.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4VGFzay5qYXZh) | `76.08% <0%> (-4.35%)` | `5% <0%> (ø)` | | | [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0%> (-0.94%)` | `24% <0%> (ø)` | | | [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `30.87% <0%> (+0.67%)` | `25% <0%> (+1%)` | :arrow_up: | | [...lin/elasticsearch/writer/FutureCallbackHolder.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3dyaXRlci9GdXR1cmVDYWxsYmFja0hvbGRlci5qYXZh) | `62.85% <0%> (+1.42%)` | `4% <0%> (ø)` | :arrow_down: | | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2788/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `72.22% <0%> (+2.22%)` | `13% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2788?src=pr&el=footer). Last update [60e3ab5...a7dcea7](https://codecov.io/gh/apache/incubator-gobbl
[GitHub] [incubator-gobblin] asfgit closed pull request #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema
asfgit closed pull request #2791: [GOBBLIN-941] Enhance DDL to add column and column.types with case-preserving schema URL: https://github.com/apache/incubator-gobblin/pull/2791 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
