[GitHub] [incubator-gobblin] jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r295596935 ## File path: conf/cluster-master/application.conf ## @@ -69,3 +70,20 @@ task.status.reportintervalinms=1000 # Enable metrics / events metrics.enabled=true +# UI +admin.server.enabled=true Review comment: @ibuenros, made default to false as before, since it requires store config as well. pls take a look. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r295596935 ## File path: conf/cluster-master/application.conf ## @@ -69,3 +70,20 @@ task.status.reportintervalinms=1000 # Enable metrics / events metrics.enabled=true +# UI +admin.server.enabled=true Review comment: @ibuenros, made default to false as before, since it requires store config 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] jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287553190 ## File path: bin/gobblin-admin.sh ## @@ -1,142 +0,0 @@ -#!/bin/bash Review comment: I was targeting to cleanup and simplify those too many scripts Gobblin has, which could be not only confusing for new users but also not a standard way of using it since all scripts have different params. user should really move to new scripts to standardize, so not sure how much the backward compatibility is required for scripts, as oppose to APIs, otherwise we will never be able to clean up and provide better version. but I see ur point so may be I will create wrapper as you suggested with note to remove those wrapper in future but i afraid it wont be 100% backward compatible since loy of things (options and features) are getting standardized here. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287553190 ## File path: bin/gobblin-admin.sh ## @@ -1,142 +0,0 @@ -#!/bin/bash Review comment: I was targeting to cleanup and simplify those too many scripts Gobblin has, which could be not only confusing for new users but also not a standard way of using it since all scripts have different params. user should really move to new scripts to standardize, so not sure how much the backward compatibility is required for scripts, as oppose to APIs, otherwise we will never be able to clean up and provide better version. but I see ur point so may be I will create wrapper as you suggested with note to remove those wrapper in future but i afraid it wont be 100% backward compatible since log of things are getting standardized here. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite
gobblin script to combine all modes and command
URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287552990
##
File path: bin/gobblin.sh
##
@@ -17,50 +17,488 @@
# limitations under the License.
#
-calling_dir() {
- echo "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+# JAVA_HOME is required.
+if [[ -z "$JAVA_HOME" ]]; then
+echo -e "\nError: Environment variable JAVA_HOME not set!\n"
+exit 1
+fi
+
+# global vars
+
+GOBBLIN_VERSION=@project.version@
+GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)"
+GOBBLIN_LIB=${GOBBLIN_HOME}/lib
+GOBBLIN_BIN=${GOBBLIN_HOME}/bin
+GOBBLIN_LOGS=${GOBBLIN_HOME}/logs
+GOBBLIN_CONF=''
+
+#sourcing basic gobblin env vars like GOBBLIN_HOME and GOBBLIN_LIB
+. ${GOBBLIN_BIN}/gobblin-env.sh
+
+CLUSTER_NAME="gobblin_cluster"
+JVM_OPTS="-Xmx1g -Xms512m"
+LOG4J_FILE_PATH=''
+LOG4J_OPTS=''
+GOBBLIN_MODE=''
+ACTION=''
+JVM_FLAGS=''
+EXTRA_JARS=''
+VERBOSE=0
+ENABLE_GC_LOGS=0
+CMD_PARAMS=''
+
+
+# Gobblin Commands, Modes & respective Classes
+GOBBLIN_MODE_TYPE=''
+CLI='cli'
+SERVICE='service'
+
+# Commands
+JOB_STATE_TO_JSON_CMD='job-state-to-json'
+JOB_STORE_SCHEMA_MANAGER_CMD='job-store-schema-manager'
+CLASSPATH_CMD='classpath'
+
+# Execution Modes
+STANDALONE_MODE='standalone'
+CLUSTER_MASTER_MODE='cluster-master'
+CLUSTER_WORKER_MODE='cluster-worker'
+AWS_MODE='aws'
+YARN_MODE='yarn'
+MAPREDUCE_MODE='mapreduce'
+SERVICE_MANAGER_MODE='service-manager'
+
+GOBBLIN_EXEC_MODE_LIST="$STANDALONE_MODE $CLUSTER_MASTER_MODE
$CLUSTER_WORKER_MODE $AWS_MODE $YARN_MODE $MAPREDUCE_MODE $SERVICE_MANAGER_MODE"
+
+# CLI Command class
+CLI_CLASS='org.apache.gobblin.runtime.cli.GobblinCli'
+
+# Service Class
+STANDALONE_CLASS='org.apache.gobblin.scheduler.SchedulerDaemon'
+CLUSTER_MASTER_CLASS='org.apache.gobblin.cluster.GobblinClusterManager'
+CLUSTER_WORKER_CLASS='org.apache.gobblin.cluster.GobblinTaskRunner'
+AWS_CLASS='org.apache.gobblin.aws.GobblinAWSClusterLauncher'
+YARN_CLASS='org.apache.gobblin.yarn.GobblinYarnAppLauncher'
+MAPREDUCE_CLASS='org.apache.gobblin.runtime.mapreduce.CliMRJobLauncher'
+SERVICE_MANAGER_CLASS='org.apache.gobblin.service.modules.core.GobblinServiceManager'
+
+
+function print_gobblin_usage() {
+echo "Usage:"
+echo "gobblin.sh cli "
+echo "gobblin.sh service "
+echo ""
+echo "Use \"gobblin --help\" for more information.
(Gobblin Version: $GOBBLIN_VERSION)"
+}
+
+function print_gobblin_cli_usage() {
Review comment:
Yes, GobblinCLI has `printusage` which will list all the Alias classes, but
it wont display other help options. Also its better to have all help options at
script level rather than at class level. Also the actual command help is going
to be managed by the implementor class and btw both are available depending on
the usage.
```
bin/gobblin cli somecommand
Could not find an application with alias somecommand
Available commands:
job-state-to-json To convert Job state to JSON
jobsCommand line job info and operations
passwordManager Encrypt or decrypt strings for the password
manager.
run Run a Gobblin application.
decrypt Decryption utilities
job-store-schema-managerDatabase job history store
schema manager
stateMigration Command line tools for migrating state store
keystoreExamine JCE Keystore files
config Query the config library
watermarks Inspect streaming watermarks
cleaner Data retention utility```
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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287553190 ## File path: bin/gobblin-admin.sh ## @@ -1,142 +0,0 @@ -#!/bin/bash Review comment: I was targeting to cleanup and simplify those too many scripts Gobblin has, which could be confusing for new users. user should really move to new scripts, so not sure how much the backward compatibility is required for scripts, as oppose to APIs, otherwise we will never be able to clean up. but I see ur point so may be I will create wrapper as you suggested with note to remove those wrapper in 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] jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287552780 ## File path: bin/gobblin.sh ## @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash Review comment: yes, i have updated the doc with updated help and sample commands, will be updating more as i make the changes, 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 a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287553190 ## File path: bin/gobblin-admin.sh ## @@ -1,142 +0,0 @@ -#!/bin/bash Review comment: I was targeting to cleanup and simplify those too many scripts Gobblin has, which could be confusing for new users. user should really move to new scripts, so not sure how much the backward compatibility is required for scripts, as oppose to APIs, otherwise we will never be able to clean up. but I see ur point so may be I will create wrapper as you suggested with note to remove that in 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] jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite
gobblin script to combine all modes and command
URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287553047
##
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/HelixRetriggeringJobCallable.java
##
@@ -152,7 +152,7 @@ public Void call() throws JobException {
GobblinClusterConfigurationKeys.JOB_ALWAYS_DELETE,
"false");
-try {
+try { //TODO: what is really the difference ?
Review comment:
oh this got into this by mistake, it was my own code comment. will remove 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] jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite
gobblin script to combine all modes and command
URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287552990
##
File path: bin/gobblin.sh
##
@@ -17,50 +17,488 @@
# limitations under the License.
#
-calling_dir() {
- echo "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+# JAVA_HOME is required.
+if [[ -z "$JAVA_HOME" ]]; then
+echo -e "\nError: Environment variable JAVA_HOME not set!\n"
+exit 1
+fi
+
+# global vars
+
+GOBBLIN_VERSION=@project.version@
+GOBBLIN_HOME="$(cd `dirname $0`/..; pwd)"
+GOBBLIN_LIB=${GOBBLIN_HOME}/lib
+GOBBLIN_BIN=${GOBBLIN_HOME}/bin
+GOBBLIN_LOGS=${GOBBLIN_HOME}/logs
+GOBBLIN_CONF=''
+
+#sourcing basic gobblin env vars like GOBBLIN_HOME and GOBBLIN_LIB
+. ${GOBBLIN_BIN}/gobblin-env.sh
+
+CLUSTER_NAME="gobblin_cluster"
+JVM_OPTS="-Xmx1g -Xms512m"
+LOG4J_FILE_PATH=''
+LOG4J_OPTS=''
+GOBBLIN_MODE=''
+ACTION=''
+JVM_FLAGS=''
+EXTRA_JARS=''
+VERBOSE=0
+ENABLE_GC_LOGS=0
+CMD_PARAMS=''
+
+
+# Gobblin Commands, Modes & respective Classes
+GOBBLIN_MODE_TYPE=''
+CLI='cli'
+SERVICE='service'
+
+# Commands
+JOB_STATE_TO_JSON_CMD='job-state-to-json'
+JOB_STORE_SCHEMA_MANAGER_CMD='job-store-schema-manager'
+CLASSPATH_CMD='classpath'
+
+# Execution Modes
+STANDALONE_MODE='standalone'
+CLUSTER_MASTER_MODE='cluster-master'
+CLUSTER_WORKER_MODE='cluster-worker'
+AWS_MODE='aws'
+YARN_MODE='yarn'
+MAPREDUCE_MODE='mapreduce'
+SERVICE_MANAGER_MODE='service-manager'
+
+GOBBLIN_EXEC_MODE_LIST="$STANDALONE_MODE $CLUSTER_MASTER_MODE
$CLUSTER_WORKER_MODE $AWS_MODE $YARN_MODE $MAPREDUCE_MODE $SERVICE_MANAGER_MODE"
+
+# CLI Command class
+CLI_CLASS='org.apache.gobblin.runtime.cli.GobblinCli'
+
+# Service Class
+STANDALONE_CLASS='org.apache.gobblin.scheduler.SchedulerDaemon'
+CLUSTER_MASTER_CLASS='org.apache.gobblin.cluster.GobblinClusterManager'
+CLUSTER_WORKER_CLASS='org.apache.gobblin.cluster.GobblinTaskRunner'
+AWS_CLASS='org.apache.gobblin.aws.GobblinAWSClusterLauncher'
+YARN_CLASS='org.apache.gobblin.yarn.GobblinYarnAppLauncher'
+MAPREDUCE_CLASS='org.apache.gobblin.runtime.mapreduce.CliMRJobLauncher'
+SERVICE_MANAGER_CLASS='org.apache.gobblin.service.modules.core.GobblinServiceManager'
+
+
+function print_gobblin_usage() {
+echo "Usage:"
+echo "gobblin.sh cli "
+echo "gobblin.sh service "
+echo ""
+echo "Use \"gobblin --help\" for more information.
(Gobblin Version: $GOBBLIN_VERSION)"
+}
+
+function print_gobblin_cli_usage() {
Review comment:
Yes, GobblinCLI has `printusage` which will list all the Alias classes, but
it wont display other help options. Also its better to have all help options at
script level rather than at class level.
btw both are available depending on the usage.
```
bin/gobblin cli somecommand
Could not find an application with alias somecommand
Available commands:
job-state-to-json To convert Job state to JSON
jobsCommand line job info and operations
passwordManager Encrypt or decrypt strings for the password
manager.
run Run a Gobblin application.
decrypt Decryption utilities
job-store-schema-managerDatabase job history store
schema manager
stateMigration Command line tools for migrating state store
keystoreExamine JCE Keystore files
config Query the config library
watermarks Inspect streaming watermarks
cleaner Data retention utility```
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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r287552780 ## File path: bin/gobblin.sh ## @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash Review comment: yes, i have updated the doc with updated help and sample commands. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275093655 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: @ibuenros , lets take this offline over email. 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 a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275089641 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: Each removed shell script has been accommodated into the updated `gobblin.sh`. if you can pull this PR and try some of the commands, it will be clear on how things are working with these updates. git diff is bit confusing for such large PR. 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 a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275089641 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: to handle it on bash script level and call the `GobblinCli` with appropriate args for each command accordingly. btw, each removed shell script has support to execute via this updated `gobblin.sh`. if you can pull this PR and try some of the commands, it will be clear on how things are working with these updates. git diff is bit confusing for such large PR. 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 a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite
gobblin script to combine all modes and command
URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275072150
##
File path: bin/gobblin.sh
##
@@ -17,50 +17,435 @@
# limitations under the License.
#
-calling_dir() {
- echo "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-}
-classpath() {
- DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-
- for i in `ls $DIR/../lib`
- do
- if [[ $i != hadoop* ]]
- then
-CLASSPATH=${CLASSPATH:+${CLASSPATH}:}$DIR/../lib/$i
- else
-
HADOOP_CLASSPATH=${HADOOP_CLASSPATH:+${HADOOP_CLASSPATH}:}$DIR/../lib/$i
- fi
- done
-
- if [ ! -z "$HADOOP_HOME" ] && [ -f $HADOOP_HOME/bin/hadoop ]
- then
-HADOOP_CLASSPATH=$($HADOOP_HOME/bin/hadoop classpath)
- fi
-
- CLASSPATH=$CLASSPATH:$HADOOP_CLASSPATH
-
- if [ ! -z "$GOBBLIN_ADDITIONAL_JARS" ]
- then
- CLASSPATH=$GOBBLIN_ADDITIONAL_JARS:$CLASSPATH
- fi
-
- echo $CLASSPATH
+# JAVA_HOME is required.
+if [[ -z "$JAVA_HOME" ]]; then
+echo -e "\nError: Environment variable JAVA_HOME not set!\n"
+exit 1
+fi
+
+# global vars
+
+GOBBLIN_VERSION="0.15.0"
Review comment:
will replace this with @project.version@ to get it placed by gradle.
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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275060881 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: ok , sure. is `gobblin admin ` fine ? is your comment only about `gobblin cli run `? then i can bring the `run`, `watermarks`, `passwrodManager`, etc commands to gobblin.sh level so that the command signature wont change and it will remain `gobblin run `. make sense? 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275060881 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: ok , sure. is `gobblin admin ` fine ? is it only about `gobblin cli run `? then i can bring the `run`, `watermarks`, `passwrodManager`, etc commands to gobblin.sh level so that the command signature wont change and it will remain `gobblin run `. make sense ? 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275060881 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: ok , sure. is `gobblin admin ` fine ? is it only about `gobblin cli run `? then i can bring the `run`, `watermarks`, `passwrodManager`, etc commands to gobblin.sh level so that the command signature wont change from existing 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] jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275056186 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: Hi @ibuenros , the `cli` has lot of options other than `run`, like `passwordManager`, `watermarks`, etc... so we can keep it under `cli` category for now, and in future, those options can be added directly to the `gobblin.sh` if we really want to have `run` command directly under gobblin without `cli`. btw, bringing all possible command line options at the `gobblin.sh` level could be too much aggregation. here the problem I m trying to solve is mostly inconsistency and lack of clarity among functionality of all gobblin scripts. I have also added more detail GOBBLIN-707 jira ticket to explain why we should combine all these scripts. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r275056186 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: Hi @ibuenros , the `cli` has lot of options other than `run`, like `passwordManager`, `watermarks`, etc... so we can keep it under `cli` category for now, and in future, those options can be added directly to the `gobblin.sh` if we really want to have `run` command directly under gobblin without `cli`. here the problem I m trying to solve is mostly inconsistency and lack of clarity among functionality of all gobblin scripts. I have also added more detail GOBBLIN-707 jira ticket to explain why we should combine all these scripts. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r272445741 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: ok sure, it requires log of doc changes, and some reorganization, which i can take care of but can we get #2586 merged? otherwise i ll have lot of conflicts. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite
gobblin script to combine all modes and command
URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r272026110
##
File path: conf/yarn/application.conf
##
@@ -22,15 +22,18 @@ gobblin.yarn.app.name=GobblinYarn
gobblin.yarn.app.master.memory.mbs=256
gobblin.yarn.initial.containers=2
gobblin.yarn.container.memory.mbs=512
-gobblin.yarn.conf.dir=
-gobblin.yarn.lib.jars.dir=
-gobblin.yarn.app.master.files.local=${gobblin.yarn.conf.dir}"/log4j-yarn.properties,"${gobblin.yarn.conf.dir}"/application.conf,"${gobblin.yarn.conf.dir}"/reference.conf"
+gobblin.yarn.conf.dir=/tools/gobblin-dist/conf/yarn/
Review comment:
this is missed, let me change this to
`gobblin.yarn.conf.dir=${GOBBLIN_HOME}/conf/yarn/` will be better than having
btw, thanks for catching this, this was my local config.
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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite
gobblin script to combine all modes and command
URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r272026110
##
File path: conf/yarn/application.conf
##
@@ -22,15 +22,18 @@ gobblin.yarn.app.name=GobblinYarn
gobblin.yarn.app.master.memory.mbs=256
gobblin.yarn.initial.containers=2
gobblin.yarn.container.memory.mbs=512
-gobblin.yarn.conf.dir=
-gobblin.yarn.lib.jars.dir=
-gobblin.yarn.app.master.files.local=${gobblin.yarn.conf.dir}"/log4j-yarn.properties,"${gobblin.yarn.conf.dir}"/application.conf,"${gobblin.yarn.conf.dir}"/reference.conf"
+gobblin.yarn.conf.dir=/tools/gobblin-dist/conf/yarn/
Review comment:
this is missed, let me change this to
`gobblin.yarn.conf.dir=${GOBBLIN_HOME}/conf/yarn/` thanks for catching this,
this was my local config.
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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r272025514 ## File path: conf/yarn/reference.conf ## @@ -38,6 +38,6 @@ gobblin.yarn.work.dir=/gobblin gobblin.cluster.helix.cluster.name=GobblinYarn gobblin.cluster.zk.connection.string="localhost:2181" -fs.uri="hdfs://localhost:9000" Review comment: yes, both are acceptable, just changing it to 8020 as default which i believe most people use, can change it to 9000 if its otherwise, no prob. 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r271550178 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: @autumnust , does that make sense ? 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r268845649 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: Hi, @autumnust, with this change, It will become valid. I am targeting to combine all services and commands together because the config, logging and other mgmt. logics are done differently for each script, which makes hard for new user to understand how to consistently run all services and commands with same experience. btw, gobblin is just soft link to gobblin.sh, it only changes the command signature than the functionality. Following becomes the new way of using gobblin script for various activities. `gobblin.sh ` `gobblin.sh ` commands values: `admin, cli, statestore-check, statestore-clean, historystore-manager` service values: `standalone, cluster-master, cluster-worker, aws, yarn, mr, service` 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r268845649 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: Hi, @autumnust, with this change, It will become valid. I am targetting to combine all services and commands together because the config and other mgmt is the same for all services and commands. since the gobblin is just link to gobblin.sh, with this change it will handle the cli as a command. so following becomes the new way of using gobblin script for various activities. `gobblin.sh ` `gobblin.sh ` commands values: `admin, cli, statestore-check, statestore-clean, historystore-manager` service values: `standalone, cluster-master, cluster-worker, aws, yarn, mr, service` 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 #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command
jhsenjaliya commented on a change in pull request #2578: [GOBBLIN-707] rewrite gobblin script to combine all modes and command URL: https://github.com/apache/incubator-gobblin/pull/2578#discussion_r268845649 ## File path: gobblin-docs/user-guide/Gobblin-CLI.md ## @@ -28,29 +28,29 @@ Gobblin ingestion applications Gobblin ingestion applications can be accessed through the command `run`: ```bash -bin/gobblin run [listQuickApps] [] -jobName [OPTIONS] +bin/gobblin cli run [listQuickApps] [] -jobName [OPTIONS] Review comment: @autumnust, with this change, I am targetting to combine all services and commands together because the config and other mgmt is the same for all services and commands. since the gobblin is just link to gobblin.sh, with this change it will handle the cli as a command. so following becomes the new way of using gobblin script for various activities. `gobblin.sh gobblin.sh commands values: admin, cli, statestore-check, statestore-clean, historystore-manager service values: standalone, cluster-master, cluster-worker, aws, yarn, mr, service` 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
