You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2022/03/31 14:45:52 UTC
[GitHub] [incubator-seatunnel] ruanwenjun opened a new pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
ruanwenjun opened a new pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636
## Purpose of this pull request
close #1632
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in you PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/development/new-license.md)
* [ ] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r840173129
##########
File path: seatunnel-core/seatunnel-core-flink/src/test/java/org/apache/seatunnel/FlinkStarterTest.java
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class FlinkStarterTest {
+
+ @Test
+ public void buildCommands() {
+ String[] args = {"--config", "test.conf", "-m", "yarn-cluster", "-i", "key1=value1", "-i", "key2=value2"};
+ FlinkStarter flinkStarter = new FlinkStarter(args);
Review comment:
Ok, I added a negative case, please review.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] yx91490 commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
yx91490 commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841203613
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/bin/start-seatunnel-flink.sh
##########
@@ -16,87 +16,24 @@
# limitations under the License.
#
-# copy command line arguments
-
-function usage() {
- echo "Usage: start-seatunnel-flink.sh [options]"
- echo " options:"
- echo " --config, -c FILE_PATH Config file"
- echo " --variable, -i PROP=VALUE Variable substitution, such as -i city=beijing, or -i date=20190318"
- echo " --check, -t Check config"
- echo " --help, -h Show this help message"
-}
-
-if [[ "$@" = *--help ]] || [[ "$@" = *-h ]] || [[ $# -le 1 ]]; then
- usage
- exit 0
-fi
-
-is_exist() {
- if [ -z $1 ]; then
- usage
- exit 1
- fi
-}
-
-PARAMS=""
-while (( "$#" )); do
- case "$1" in
- -c|--config)
- CONFIG_FILE=$2
- is_exist ${CONFIG_FILE}
- shift 2
- ;;
-
- -i|--variable)
- variable=$2
- is_exist ${variable}
- java_property_value="-D${variable}"
- variables_substitution="${java_property_value} ${variables_substitution}"
- shift 2
- ;;
-
- *) # preserve positional arguments
- PARAMS="$PARAMS $1"
- shift
- ;;
-
- esac
-done
-
-if [ -z ${CONFIG_FILE} ]; then
- echo "Error: The following option is required: [-c | --config]"
- usage
- exit -1
-elif [ ! -f ${CONFIG_FILE} ];then
- echo "Error: Config file ${CONFIG_FILE} does not exists! Please check it."
- exit -1
-fi
-
-# set positional arguments in their proper place
-eval set -- "$PARAMS"
-
-BIN_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-APP_DIR=$(dirname ${BIN_DIR})
+set -eu
+APP_DIR=$(cd $(dirname ${0})/../;pwd)
CONF_DIR=${APP_DIR}/config
-PLUGINS_DIR=${APP_DIR}/lib
-DEFAULT_CONFIG=${CONF_DIR}/application.conf
-CONFIG_FILE=${CONFIG_FILE:-$DEFAULT_CONFIG}
-
-assemblyJarName=$(find ${PLUGINS_DIR} -name seatunnel-core-flink*.jar)
+APP_JAR=${APP_DIR}/lib/seatunnel-core-flink.jar
if [ -f "${CONF_DIR}/seatunnel-env.sh" ]; then
- source ${CONF_DIR}/seatunnel-env.sh
+ . "${CONF_DIR}/seatunnel-env.sh"
fi
-string_trim() {
- echo $1 | awk '{$1=$1;print}'
-}
-
-JVM_ARGS=$(string_trim "${variables_substitution}")
Review comment:
you may mistake my meaning :(
your code don't set JVM_ARGS environment variable, that's the difference.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] yx91490 commented on pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
yx91490 commented on pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#issuecomment-1086873734
LGTM
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] yx91490 removed a comment on pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
yx91490 removed a comment on pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#issuecomment-1086873734
LGTM
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841148535
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/java/org/apache/seatunnel/FlinkStarter.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.apache.seatunnel.command.FlinkCommandArgs;
+import org.apache.seatunnel.common.config.Common;
+
+import com.beust.jcommander.JCommander;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * The SeaTunnel flink starter. This class is responsible for generate the final flink job execute command.
+ */
+public class FlinkStarter implements Starter {
+
+ private static final String APP_NAME = SeatunnelFlink.class.getName();
+ private static final int USAGE_EXIT_CODE = 234;
+ private static final String APP_JAR_NAME = "seatunnel-core-flink.jar";
+
+ /**
+ * Flink parameters, used by flink job itself. e.g. `-m yarn-cluster`
+ */
+ private final List<String> flinkParams = new ArrayList<>();
+
+ /**
+ * SeaTunnel parameters, used by SeaTunnel application. e.g. `-c config.conf`
+ */
+ private final FlinkCommandArgs flinkCommandArgs;
+
+ /**
+ * SeaTunnel flink job jar.
+ */
+ private final String appJar;
+
+ public FlinkStarter(String[] args) {
Review comment:
Use public here is just used for test, I removed the public here, and use the default scope.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841148627
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/bin/start-seatunnel-flink.sh
##########
@@ -16,87 +16,24 @@
# limitations under the License.
#
-# copy command line arguments
-
-function usage() {
- echo "Usage: start-seatunnel-flink.sh [options]"
- echo " options:"
- echo " --config, -c FILE_PATH Config file"
- echo " --variable, -i PROP=VALUE Variable substitution, such as -i city=beijing, or -i date=20190318"
- echo " --check, -t Check config"
- echo " --help, -h Show this help message"
-}
-
-if [[ "$@" = *--help ]] || [[ "$@" = *-h ]] || [[ $# -le 1 ]]; then
- usage
- exit 0
-fi
-
-is_exist() {
- if [ -z $1 ]; then
- usage
- exit 1
- fi
-}
-
-PARAMS=""
-while (( "$#" )); do
- case "$1" in
- -c|--config)
- CONFIG_FILE=$2
- is_exist ${CONFIG_FILE}
- shift 2
- ;;
-
- -i|--variable)
- variable=$2
- is_exist ${variable}
- java_property_value="-D${variable}"
- variables_substitution="${java_property_value} ${variables_substitution}"
- shift 2
- ;;
-
- *) # preserve positional arguments
- PARAMS="$PARAMS $1"
- shift
- ;;
-
- esac
-done
-
-if [ -z ${CONFIG_FILE} ]; then
- echo "Error: The following option is required: [-c | --config]"
- usage
- exit -1
-elif [ ! -f ${CONFIG_FILE} ];then
- echo "Error: Config file ${CONFIG_FILE} does not exists! Please check it."
- exit -1
-fi
-
-# set positional arguments in their proper place
-eval set -- "$PARAMS"
-
-BIN_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-APP_DIR=$(dirname ${BIN_DIR})
+set -eu
+APP_DIR=$(cd $(dirname ${0})/../;pwd)
CONF_DIR=${APP_DIR}/config
-PLUGINS_DIR=${APP_DIR}/lib
-DEFAULT_CONFIG=${CONF_DIR}/application.conf
-CONFIG_FILE=${CONFIG_FILE:-$DEFAULT_CONFIG}
-
-assemblyJarName=$(find ${PLUGINS_DIR} -name seatunnel-core-flink*.jar)
+APP_JAR=${APP_DIR}/lib/seatunnel-core-flink.jar
if [ -f "${CONF_DIR}/seatunnel-env.sh" ]; then
- source ${CONF_DIR}/seatunnel-env.sh
+ . "${CONF_DIR}/seatunnel-env.sh"
fi
-string_trim() {
- echo $1 | awk '{$1=$1;print}'
-}
-
-JVM_ARGS=$(string_trim "${variables_substitution}")
Review comment:
Yes, it's better to use trim to process the variables. Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] yx91490 commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
yx91490 commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841224828
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/bin/start-seatunnel-flink.sh
##########
@@ -16,87 +16,24 @@
# limitations under the License.
#
-# copy command line arguments
-
-function usage() {
- echo "Usage: start-seatunnel-flink.sh [options]"
- echo " options:"
- echo " --config, -c FILE_PATH Config file"
- echo " --variable, -i PROP=VALUE Variable substitution, such as -i city=beijing, or -i date=20190318"
- echo " --check, -t Check config"
- echo " --help, -h Show this help message"
-}
-
-if [[ "$@" = *--help ]] || [[ "$@" = *-h ]] || [[ $# -le 1 ]]; then
- usage
- exit 0
-fi
-
-is_exist() {
- if [ -z $1 ]; then
- usage
- exit 1
- fi
-}
-
-PARAMS=""
-while (( "$#" )); do
- case "$1" in
- -c|--config)
- CONFIG_FILE=$2
- is_exist ${CONFIG_FILE}
- shift 2
- ;;
-
- -i|--variable)
- variable=$2
- is_exist ${variable}
- java_property_value="-D${variable}"
- variables_substitution="${java_property_value} ${variables_substitution}"
- shift 2
- ;;
-
- *) # preserve positional arguments
- PARAMS="$PARAMS $1"
- shift
- ;;
-
- esac
-done
-
-if [ -z ${CONFIG_FILE} ]; then
- echo "Error: The following option is required: [-c | --config]"
- usage
- exit -1
-elif [ ! -f ${CONFIG_FILE} ];then
- echo "Error: Config file ${CONFIG_FILE} does not exists! Please check it."
- exit -1
-fi
-
-# set positional arguments in their proper place
-eval set -- "$PARAMS"
-
-BIN_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-APP_DIR=$(dirname ${BIN_DIR})
+set -eu
+APP_DIR=$(cd $(dirname ${0})/../;pwd)
CONF_DIR=${APP_DIR}/config
-PLUGINS_DIR=${APP_DIR}/lib
-DEFAULT_CONFIG=${CONF_DIR}/application.conf
-CONFIG_FILE=${CONFIG_FILE:-$DEFAULT_CONFIG}
-
-assemblyJarName=$(find ${PLUGINS_DIR} -name seatunnel-core-flink*.jar)
+APP_JAR=${APP_DIR}/lib/seatunnel-core-flink.jar
if [ -f "${CONF_DIR}/seatunnel-env.sh" ]; then
- source ${CONF_DIR}/seatunnel-env.sh
+ . "${CONF_DIR}/seatunnel-env.sh"
fi
-string_trim() {
- echo $1 | awk '{$1=$1;print}'
-}
-
-JVM_ARGS=$(string_trim "${variables_substitution}")
Review comment:
OK
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841148547
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/java/org/apache/seatunnel/FlinkStarter.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.apache.seatunnel.command.FlinkCommandArgs;
+import org.apache.seatunnel.common.config.Common;
+
+import com.beust.jcommander.JCommander;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * The SeaTunnel flink starter. This class is responsible for generate the final flink job execute command.
+ */
+public class FlinkStarter implements Starter {
+
+ private static final String APP_NAME = SeatunnelFlink.class.getName();
+ private static final int USAGE_EXIT_CODE = 234;
+ private static final String APP_JAR_NAME = "seatunnel-core-flink.jar";
+
+ /**
+ * Flink parameters, used by flink job itself. e.g. `-m yarn-cluster`
+ */
+ private final List<String> flinkParams = new ArrayList<>();
+
+ /**
+ * SeaTunnel parameters, used by SeaTunnel application. e.g. `-c config.conf`
+ */
+ private final FlinkCommandArgs flinkCommandArgs;
+
+ /**
+ * SeaTunnel flink job jar.
+ */
+ private final String appJar;
+
+ public FlinkStarter(String[] args) {
+ this.flinkCommandArgs = parseArgs(args);
+ // set the deployment mode, used to get the job jar path.
+ Common.setDeployMode(flinkCommandArgs.getDeployMode().getName());
+ this.appJar = Common.appLibDir().resolve(APP_JAR_NAME).toString();
+ }
+
+ @SuppressWarnings("checkstyle:RegexpSingleline")
+ public static void main(String[] args) {
+ FlinkStarter flinkStarter = new FlinkStarter(args);
+ List<String> command = flinkStarter.buildCommands();
+ String finalFLinkCommand = String.join(" ", command);
+ System.out.println(finalFLinkCommand);
+ }
+
+ /**
+ * Parse seatunnel args.
+ *
+ * @param args args
+ * @return FlinkCommandArgs
+ */
+ private FlinkCommandArgs parseArgs(String[] args) {
+ FlinkCommandArgs flinkCommandArgs = new FlinkCommandArgs();
+ JCommander jCommander = JCommander.newBuilder()
+ .programName("start-seatunnel-flink.sh")
+ .addObject(flinkCommandArgs)
+ .acceptUnknownOptions(true)
+ .args(args)
+ .build();
+ // The args is not belongs to seatunnel, add into flink params
+ flinkParams.addAll(jCommander.getUnknownOptions());
+ if (flinkCommandArgs.isHelp()) {
+ jCommander.usage();
+ System.exit(USAGE_EXIT_CODE);
+ }
+ return flinkCommandArgs;
+ }
+
+ @Override
+ public List<String> buildCommands() {
+ List<String> command = new ArrayList<>();
+ command.add("${FLINK_HOME}/bin/flink run");
+ command.addAll(flinkParams);
+ command.add("-c " + APP_NAME);
+ command.add(appJar);
+ command.add("--config " + flinkCommandArgs.getConfigFile());
Review comment:
Ok, modified as you suggested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] simon824 commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
simon824 commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r840152780
##########
File path: seatunnel-core/seatunnel-core-flink/src/test/java/org/apache/seatunnel/FlinkStarterTest.java
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class FlinkStarterTest {
+
+ @Test
+ public void buildCommands() {
+ String[] args = {"--config", "test.conf", "-m", "yarn-cluster", "-i", "key1=value1", "-i", "key2=value2"};
+ FlinkStarter flinkStarter = new FlinkStarter(args);
Review comment:
can we test a negative case, like missing `config` param
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#issuecomment-1084707244
@simon824 PTAL, 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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841207702
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/java/org/apache/seatunnel/FlinkStarter.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.apache.seatunnel.command.FlinkCommandArgs;
+import org.apache.seatunnel.common.config.Common;
+
+import com.beust.jcommander.JCommander;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * The SeaTunnel flink starter. This class is responsible for generate the final flink job execute command.
+ */
+public class FlinkStarter implements Starter {
+
+ private static final String APP_NAME = SeatunnelFlink.class.getName();
+ private static final int USAGE_EXIT_CODE = 234;
+ private static final String APP_JAR_NAME = "seatunnel-core-flink.jar";
+
+ /**
+ * Flink parameters, used by flink job itself. e.g. `-m yarn-cluster`
+ */
+ private final List<String> flinkParams = new ArrayList<>();
+
+ /**
+ * SeaTunnel parameters, used by SeaTunnel application. e.g. `-c config.conf`
+ */
+ private final FlinkCommandArgs flinkCommandArgs;
+
+ /**
+ * SeaTunnel flink job jar.
+ */
+ private final String appJar;
+
+ FlinkStarter(String[] args) {
+ this.flinkCommandArgs = parseArgs(args);
+ // set the deployment mode, used to get the job jar path.
+ Common.setDeployMode(flinkCommandArgs.getDeployMode().getName());
+ this.appJar = Common.appLibDir().resolve(APP_JAR_NAME).toString();
+ }
+
+ @SuppressWarnings("checkstyle:RegexpSingleline")
+ public static void main(String[] args) {
+ FlinkStarter flinkStarter = new FlinkStarter(args);
+ List<String> command = flinkStarter.buildCommands();
+ String finalFLinkCommand = String.join(" ", command);
+ System.out.println(finalFLinkCommand);
+ }
+
+ /**
+ * Parse seatunnel args.
+ *
+ * @param args args
+ * @return FlinkCommandArgs
+ */
+ private FlinkCommandArgs parseArgs(String[] args) {
+ FlinkCommandArgs flinkCommandArgs = new FlinkCommandArgs();
+ JCommander jCommander = JCommander.newBuilder()
+ .programName("start-seatunnel-flink.sh")
+ .addObject(flinkCommandArgs)
+ .acceptUnknownOptions(true)
+ .args(args)
+ .build();
+ // The args is not belongs to seatunnel, add into flink params
+ flinkParams.addAll(jCommander.getUnknownOptions());
+ if (flinkCommandArgs.isHelp()) {
+ jCommander.usage();
+ System.exit(USAGE_EXIT_CODE);
+ }
+ return flinkCommandArgs;
+ }
+
+ @Override
+ public List<String> buildCommands() {
+ List<String> command = new ArrayList<>();
+ command.add("${FLINK_HOME}/bin/flink run");
Review comment:
Done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841207590
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/bin/start-seatunnel-flink.sh
##########
@@ -16,87 +16,24 @@
# limitations under the License.
#
-# copy command line arguments
-
-function usage() {
- echo "Usage: start-seatunnel-flink.sh [options]"
- echo " options:"
- echo " --config, -c FILE_PATH Config file"
- echo " --variable, -i PROP=VALUE Variable substitution, such as -i city=beijing, or -i date=20190318"
- echo " --check, -t Check config"
- echo " --help, -h Show this help message"
-}
-
-if [[ "$@" = *--help ]] || [[ "$@" = *-h ]] || [[ $# -le 1 ]]; then
- usage
- exit 0
-fi
-
-is_exist() {
- if [ -z $1 ]; then
- usage
- exit 1
- fi
-}
-
-PARAMS=""
-while (( "$#" )); do
- case "$1" in
- -c|--config)
- CONFIG_FILE=$2
- is_exist ${CONFIG_FILE}
- shift 2
- ;;
-
- -i|--variable)
- variable=$2
- is_exist ${variable}
- java_property_value="-D${variable}"
- variables_substitution="${java_property_value} ${variables_substitution}"
- shift 2
- ;;
-
- *) # preserve positional arguments
- PARAMS="$PARAMS $1"
- shift
- ;;
-
- esac
-done
-
-if [ -z ${CONFIG_FILE} ]; then
- echo "Error: The following option is required: [-c | --config]"
- usage
- exit -1
-elif [ ! -f ${CONFIG_FILE} ];then
- echo "Error: Config file ${CONFIG_FILE} does not exists! Please check it."
- exit -1
-fi
-
-# set positional arguments in their proper place
-eval set -- "$PARAMS"
-
-BIN_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-APP_DIR=$(dirname ${BIN_DIR})
+set -eu
+APP_DIR=$(cd $(dirname ${0})/../;pwd)
CONF_DIR=${APP_DIR}/config
-PLUGINS_DIR=${APP_DIR}/lib
-DEFAULT_CONFIG=${CONF_DIR}/application.conf
-CONFIG_FILE=${CONFIG_FILE:-$DEFAULT_CONFIG}
-
-assemblyJarName=$(find ${PLUGINS_DIR} -name seatunnel-core-flink*.jar)
+APP_JAR=${APP_DIR}/lib/seatunnel-core-flink.jar
if [ -f "${CONF_DIR}/seatunnel-env.sh" ]; then
- source ${CONF_DIR}/seatunnel-env.sh
+ . "${CONF_DIR}/seatunnel-env.sh"
fi
-string_trim() {
- echo $1 | awk '{$1=$1;print}'
-}
-
-JVM_ARGS=$(string_trim "${variables_substitution}")
Review comment:
The variables have been set by `-D`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] simon824 commented on pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
simon824 commented on pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#issuecomment-1086448936
Overall LGTM, also cc @yx91490
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] yx91490 commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
yx91490 commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841137587
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/bin/start-seatunnel-flink.sh
##########
@@ -16,87 +16,24 @@
# limitations under the License.
#
-# copy command line arguments
-
-function usage() {
- echo "Usage: start-seatunnel-flink.sh [options]"
- echo " options:"
- echo " --config, -c FILE_PATH Config file"
- echo " --variable, -i PROP=VALUE Variable substitution, such as -i city=beijing, or -i date=20190318"
- echo " --check, -t Check config"
- echo " --help, -h Show this help message"
-}
-
-if [[ "$@" = *--help ]] || [[ "$@" = *-h ]] || [[ $# -le 1 ]]; then
- usage
- exit 0
-fi
-
-is_exist() {
- if [ -z $1 ]; then
- usage
- exit 1
- fi
-}
-
-PARAMS=""
-while (( "$#" )); do
- case "$1" in
- -c|--config)
- CONFIG_FILE=$2
- is_exist ${CONFIG_FILE}
- shift 2
- ;;
-
- -i|--variable)
- variable=$2
- is_exist ${variable}
- java_property_value="-D${variable}"
- variables_substitution="${java_property_value} ${variables_substitution}"
- shift 2
- ;;
-
- *) # preserve positional arguments
- PARAMS="$PARAMS $1"
- shift
- ;;
-
- esac
-done
-
-if [ -z ${CONFIG_FILE} ]; then
- echo "Error: The following option is required: [-c | --config]"
- usage
- exit -1
-elif [ ! -f ${CONFIG_FILE} ];then
- echo "Error: Config file ${CONFIG_FILE} does not exists! Please check it."
- exit -1
-fi
-
-# set positional arguments in their proper place
-eval set -- "$PARAMS"
-
-BIN_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-APP_DIR=$(dirname ${BIN_DIR})
+set -eu
+APP_DIR=$(cd $(dirname ${0})/../;pwd)
CONF_DIR=${APP_DIR}/config
-PLUGINS_DIR=${APP_DIR}/lib
-DEFAULT_CONFIG=${CONF_DIR}/application.conf
-CONFIG_FILE=${CONFIG_FILE:-$DEFAULT_CONFIG}
-
-assemblyJarName=$(find ${PLUGINS_DIR} -name seatunnel-core-flink*.jar)
+APP_JAR=${APP_DIR}/lib/seatunnel-core-flink.jar
if [ -f "${CONF_DIR}/seatunnel-env.sh" ]; then
- source ${CONF_DIR}/seatunnel-env.sh
+ . "${CONF_DIR}/seatunnel-env.sh"
fi
-string_trim() {
- echo $1 | awk '{$1=$1;print}'
-}
-
-JVM_ARGS=$(string_trim "${variables_substitution}")
Review comment:
you changed the JVM_ARGS usage method, I don't known whether will cause problem:(
especially if variables contains quote, blank space, etc.
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/java/org/apache/seatunnel/FlinkStarter.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.apache.seatunnel.command.FlinkCommandArgs;
+import org.apache.seatunnel.common.config.Common;
+
+import com.beust.jcommander.JCommander;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * The SeaTunnel flink starter. This class is responsible for generate the final flink job execute command.
+ */
+public class FlinkStarter implements Starter {
+
+ private static final String APP_NAME = SeatunnelFlink.class.getName();
+ private static final int USAGE_EXIT_CODE = 234;
+ private static final String APP_JAR_NAME = "seatunnel-core-flink.jar";
+
+ /**
+ * Flink parameters, used by flink job itself. e.g. `-m yarn-cluster`
+ */
+ private final List<String> flinkParams = new ArrayList<>();
+
+ /**
+ * SeaTunnel parameters, used by SeaTunnel application. e.g. `-c config.conf`
+ */
+ private final FlinkCommandArgs flinkCommandArgs;
+
+ /**
+ * SeaTunnel flink job jar.
+ */
+ private final String appJar;
+
+ public FlinkStarter(String[] args) {
Review comment:
can be private constructor
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/java/org/apache/seatunnel/FlinkStarter.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.apache.seatunnel.command.FlinkCommandArgs;
+import org.apache.seatunnel.common.config.Common;
+
+import com.beust.jcommander.JCommander;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * The SeaTunnel flink starter. This class is responsible for generate the final flink job execute command.
+ */
+public class FlinkStarter implements Starter {
+
+ private static final String APP_NAME = SeatunnelFlink.class.getName();
+ private static final int USAGE_EXIT_CODE = 234;
+ private static final String APP_JAR_NAME = "seatunnel-core-flink.jar";
+
+ /**
+ * Flink parameters, used by flink job itself. e.g. `-m yarn-cluster`
+ */
+ private final List<String> flinkParams = new ArrayList<>();
+
+ /**
+ * SeaTunnel parameters, used by SeaTunnel application. e.g. `-c config.conf`
+ */
+ private final FlinkCommandArgs flinkCommandArgs;
+
+ /**
+ * SeaTunnel flink job jar.
+ */
+ private final String appJar;
+
+ public FlinkStarter(String[] args) {
+ this.flinkCommandArgs = parseArgs(args);
+ // set the deployment mode, used to get the job jar path.
+ Common.setDeployMode(flinkCommandArgs.getDeployMode().getName());
+ this.appJar = Common.appLibDir().resolve(APP_JAR_NAME).toString();
+ }
+
+ @SuppressWarnings("checkstyle:RegexpSingleline")
+ public static void main(String[] args) {
+ FlinkStarter flinkStarter = new FlinkStarter(args);
+ List<String> command = flinkStarter.buildCommands();
+ String finalFLinkCommand = String.join(" ", command);
+ System.out.println(finalFLinkCommand);
+ }
+
+ /**
+ * Parse seatunnel args.
+ *
+ * @param args args
+ * @return FlinkCommandArgs
+ */
+ private FlinkCommandArgs parseArgs(String[] args) {
+ FlinkCommandArgs flinkCommandArgs = new FlinkCommandArgs();
+ JCommander jCommander = JCommander.newBuilder()
+ .programName("start-seatunnel-flink.sh")
+ .addObject(flinkCommandArgs)
+ .acceptUnknownOptions(true)
+ .args(args)
+ .build();
+ // The args is not belongs to seatunnel, add into flink params
+ flinkParams.addAll(jCommander.getUnknownOptions());
+ if (flinkCommandArgs.isHelp()) {
+ jCommander.usage();
+ System.exit(USAGE_EXIT_CODE);
+ }
+ return flinkCommandArgs;
+ }
+
+ @Override
+ public List<String> buildCommands() {
+ List<String> command = new ArrayList<>();
+ command.add("${FLINK_HOME}/bin/flink run");
+ command.addAll(flinkParams);
+ command.add("-c " + APP_NAME);
+ command.add(appJar);
+ command.add("--config " + flinkCommandArgs.getConfigFile());
Review comment:
use '+' maybe make command not atomic, split to 2 add step may be better.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] yx91490 commented on a change in pull request #1636: [Feature][Flink] Rewrite start-seatunnel-flink script with starter
Posted by GitBox <gi...@apache.org>.
yx91490 commented on a change in pull request #1636:
URL: https://github.com/apache/incubator-seatunnel/pull/1636#discussion_r841203380
##########
File path: seatunnel-core/seatunnel-core-flink/src/main/java/org/apache/seatunnel/FlinkStarter.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.seatunnel;
+
+import org.apache.seatunnel.command.FlinkCommandArgs;
+import org.apache.seatunnel.common.config.Common;
+
+import com.beust.jcommander.JCommander;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * The SeaTunnel flink starter. This class is responsible for generate the final flink job execute command.
+ */
+public class FlinkStarter implements Starter {
+
+ private static final String APP_NAME = SeatunnelFlink.class.getName();
+ private static final int USAGE_EXIT_CODE = 234;
+ private static final String APP_JAR_NAME = "seatunnel-core-flink.jar";
+
+ /**
+ * Flink parameters, used by flink job itself. e.g. `-m yarn-cluster`
+ */
+ private final List<String> flinkParams = new ArrayList<>();
+
+ /**
+ * SeaTunnel parameters, used by SeaTunnel application. e.g. `-c config.conf`
+ */
+ private final FlinkCommandArgs flinkCommandArgs;
+
+ /**
+ * SeaTunnel flink job jar.
+ */
+ private final String appJar;
+
+ FlinkStarter(String[] args) {
+ this.flinkCommandArgs = parseArgs(args);
+ // set the deployment mode, used to get the job jar path.
+ Common.setDeployMode(flinkCommandArgs.getDeployMode().getName());
+ this.appJar = Common.appLibDir().resolve(APP_JAR_NAME).toString();
+ }
+
+ @SuppressWarnings("checkstyle:RegexpSingleline")
+ public static void main(String[] args) {
+ FlinkStarter flinkStarter = new FlinkStarter(args);
+ List<String> command = flinkStarter.buildCommands();
+ String finalFLinkCommand = String.join(" ", command);
+ System.out.println(finalFLinkCommand);
+ }
+
+ /**
+ * Parse seatunnel args.
+ *
+ * @param args args
+ * @return FlinkCommandArgs
+ */
+ private FlinkCommandArgs parseArgs(String[] args) {
+ FlinkCommandArgs flinkCommandArgs = new FlinkCommandArgs();
+ JCommander jCommander = JCommander.newBuilder()
+ .programName("start-seatunnel-flink.sh")
+ .addObject(flinkCommandArgs)
+ .acceptUnknownOptions(true)
+ .args(args)
+ .build();
+ // The args is not belongs to seatunnel, add into flink params
+ flinkParams.addAll(jCommander.getUnknownOptions());
+ if (flinkCommandArgs.isHelp()) {
+ jCommander.usage();
+ System.exit(USAGE_EXIT_CODE);
+ }
+ return flinkCommandArgs;
+ }
+
+ @Override
+ public List<String> buildCommands() {
+ List<String> command = new ArrayList<>();
+ command.add("${FLINK_HOME}/bin/flink run");
Review comment:
same
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org