imbajin commented on code in PR #785:
URL:
https://github.com/apache/hugegraph-toolchain/pull/785#discussion_r4207346153
##########
hugegraph-loader/assembly/static/bin/get-params.sh:
##########
@@ -17,40 +17,145 @@
#
function get_params() {
- echo "params: $*"
- ENGINE_PARAMS=""
- HUGEGRAPH_PARAMS=""
+ ENGINE_ARGS=()
+ HUGEGRAPH_ARGS=()
+ local file="" mode="" option="" value="" files="" file_set=false
files_set=false inline=false
+ local conf_mode=""
while (("$#")); do
- case "$1" in
- --graph | --schema | --host | --port | --username | --token | --protocol
| \
+ option=${1%%=*}
+ inline=false
+ if [[ "$1" == *=* ]]; then
+ inline=true
+ value=${1#*=}
+ fi
+ case "$option" in
+ -g | --graph | -s | --schema | -h | -i | --host | -p | --port |
--username | --password | --token | --protocol | \
+ --pd-peers | --pd-token | --meta-endpoints | --direct | --route-type |
--cluster | --graphspace | --create-graph | \
--trust-store-file | --trust-store-password | --clear-all-data |
--clear-timeout | \
--incremental-mode | --failure-mode | --batch-insert-threads |
--single-insert-threads | \
- --max-conn | --max-conn-per-route | --batch-size | --max-parse-errors |
--max-insert-errors | \
+ --max-conn | --max-conn-per-route | --batch-size | --parallel-count |
--parser-threads | \
+ --start-file | --end-file | --scatter-sources | --cdc-flush-interval |
--cdc-sink-parallelism | \
+ --max-read-errors | --max-parse-errors | --max-insert-errors |
--max-read-lines | \
--timeout | --shutdown-timeout | --retry-times | --retry-interval |
--check-vertex | \
- --print-progress | --dry-run | --sink-type | --vertex-partitions |
--edge-partitions | --help )
- HUGEGRAPH_PARAMS="$HUGEGRAPH_PARAMS $1 $2"
- shift 2
+ --print-progress | --dry-run | --test-mode | --use-prefilter |
--short-id | --vertex-edge-limit | \
+ --sink-type | --vertex-partitions | --edge-partitions |
--vertex-table-name | --edge-table-name | \
+ --hbase-zk-quorum | --hbase-zk-port | --hbase-zk-parent | --restore |
--backend | --serializer | \
+ --scheduler-type | --batch-failure-fallback)
+ # LoadOptions uses arity=1 even for booleans; ShortIdConfig lists also
consume one value.
+ if [[ "$inline" == true ]]; then
+ shift
+ else
+ if (( $# < 2 )); then
+ echo "Missing value for $option" >&2
+ return 2
Review Comment:
⚠️ Blocking: no. Summary: This parser returns status 2 when an option value
is missing, but the Flink CDC launcher ignores that status and continues to
execute Flink with the rejected option omitted. Please propagate `get_params`
failures before constructing or executing the command. Evidence: exact-head
`hugegraph-flinkcdc-loader.sh:25-35` calls `get_params "$@"` without checking
its result and does not enable `set -e`.
##########
hugegraph-loader/assembly/static/bin/get-params.sh:
##########
@@ -17,40 +17,145 @@
#
function get_params() {
- echo "params: $*"
- ENGINE_PARAMS=""
- HUGEGRAPH_PARAMS=""
+ ENGINE_ARGS=()
+ HUGEGRAPH_ARGS=()
+ local file="" mode="" option="" value="" files="" file_set=false
files_set=false inline=false
+ local conf_mode=""
while (("$#")); do
- case "$1" in
- --graph | --schema | --host | --port | --username | --token | --protocol
| \
+ option=${1%%=*}
+ inline=false
+ if [[ "$1" == *=* ]]; then
+ inline=true
+ value=${1#*=}
+ fi
+ case "$option" in
+ -g | --graph | -s | --schema | -h | -i | --host | -p | --port |
--username | --password | --token | --protocol | \
+ --pd-peers | --pd-token | --meta-endpoints | --direct | --route-type |
--cluster | --graphspace | --create-graph | \
--trust-store-file | --trust-store-password | --clear-all-data |
--clear-timeout | \
--incremental-mode | --failure-mode | --batch-insert-threads |
--single-insert-threads | \
- --max-conn | --max-conn-per-route | --batch-size | --max-parse-errors |
--max-insert-errors | \
+ --max-conn | --max-conn-per-route | --batch-size | --parallel-count |
--parser-threads | \
+ --start-file | --end-file | --scatter-sources | --cdc-flush-interval |
--cdc-sink-parallelism | \
+ --max-read-errors | --max-parse-errors | --max-insert-errors |
--max-read-lines | \
--timeout | --shutdown-timeout | --retry-times | --retry-interval |
--check-vertex | \
- --print-progress | --dry-run | --sink-type | --vertex-partitions |
--edge-partitions | --help )
- HUGEGRAPH_PARAMS="$HUGEGRAPH_PARAMS $1 $2"
- shift 2
+ --print-progress | --dry-run | --test-mode | --use-prefilter |
--short-id | --vertex-edge-limit | \
+ --sink-type | --vertex-partitions | --edge-partitions |
--vertex-table-name | --edge-table-name | \
+ --hbase-zk-quorum | --hbase-zk-port | --hbase-zk-parent | --restore |
--backend | --serializer | \
+ --scheduler-type | --batch-failure-fallback)
+ # LoadOptions uses arity=1 even for booleans; ShortIdConfig lists also
consume one value.
+ if [[ "$inline" == true ]]; then
+ shift
+ else
+ if (( $# < 2 )); then
+ echo "Missing value for $option" >&2
+ return 2
+ fi
+ value=$2
+ shift 2
+ fi
+ HUGEGRAPH_ARGS+=("$option" "$value")
;;
- --file)
- file=$2
- shift 2
+ -help | --help)
+ if [[ "$inline" == true ]]; then
+ echo "Option $option does not take a value" >&2
+ return 2
+ fi
+ HUGEGRAPH_ARGS+=("$option")
+ shift
+ ;;
+ -f | --file | --deploy-mode)
+ if [[ "$inline" == true ]]; then
+ shift
+ else
+ if (( $# < 2 )); then
+ echo "Missing value for $option" >&2
+ return 2
+ fi
+ value=$2
+ shift 2
+ fi
+ if [[ "$option" == '--file' || "$option" == '-f' ]]; then
+ file=$value
+ file_set=true
+ else
+ mode=$value
+ ENGINE_ARGS+=("$option" "$value")
+ fi
+ ;;
+ --files)
+ if [[ "$inline" == true ]]; then
+ shift
+ else
+ if (( $# < 2 )); then
+ echo "Missing value for $option" >&2
+ return 2
+ fi
+ value=$2
+ shift 2
+ fi
+ if [[ -z "$files" ]]; then
+ files=$value
+ elif [[ -n "$value" ]]; then
+ files+=",$value"
+ fi
+ files_set=true
;;
- --deploy-mode)
- mode=$2
- ENGINE_PARAMS="$ENGINE_PARAMS $1 $2"
- shift 2
+ --archives | --class | --conf | -c | --driver-class-path |
--driver-cores | --driver-java-options | \
+ --driver-library-path | --driver-memory | --executor-cores |
--executor-memory | --jars | \
+ --keytab | --kill | --master | --remote | --name | --num-executors |
--packages | --exclude-packages | \
+ --principal | --properties-file | --proxy-user | --py-files | --queue |
--repositories | --status | \
+ --total-executor-cores)
+ # Consume Spark's value with its option so Loader-looking values stay
engine arguments.
+ if [[ "$inline" == true ]]; then
+ ENGINE_ARGS+=("$1")
+ shift
+ else
+ if (( $# < 2 )); then
+ echo "Missing value for $option" >&2
+ return 2
+ fi
+ value=$2
+ ENGINE_ARGS+=("$1" "$2")
+ shift 2
+ fi
+ if [[ "$option" == --conf || ( "$option" == -c && "$inline" == false )
]]; then
+ case "$value" in
+ spark.submit.deployMode=*) conf_mode=${value#*=} ;;
+ esac
+ fi
;;
- *) # preserve positional arguments
- ENGINE_PARAMS="$ENGINE_PARAMS $1"
+ *)
+ ENGINE_ARGS+=("$1")
shift
;;
esac
done
- if [ "$mode" = 'cluster' ];then
- HUGEGRAPH_PARAMS="$HUGEGRAPH_PARAMS --file ${file##*/}"
- ENGINE_PARAMS="$ENGINE_PARAMS --files ${file}"
- else
- HUGEGRAPH_PARAMS="$HUGEGRAPH_PARAMS --file ${file}"
+ # Spark's explicit --deploy-mode takes precedence over its --conf property.
+ mode=${mode:-$conf_mode}
+ if [[ "$file_set" == true ]]; then
+ if [ "$mode" = 'cluster' ]; then
+ if [[ "$file" == *#* || "$file" == *,* ]]; then
+ echo "Cluster --file must be a local mapping path without commas or #
fragments" >&2
+ return 2
+ fi
+ HUGEGRAPH_ARGS+=(--file "${file##*/}")
+ if [[ -z "$files" ]]; then
+ files=$file
+ else
+ files+=",$file"
Review Comment:
⚠️ Blocking: no. Summary: Cluster mode appends the mapping file to the
caller's `--files` list, then addresses it by basename. If another resource has
the same basename, Spark 3.5.8's YARN client rejects the duplicate and
submission fails. Please detect this collision and return a clear error, or
stage the mapping under a unique name. Evidence: Spark 3.5.8 rejects repeated
distributed filenames in
[Client.scala](https://github.com/apache/spark/blob/v3.5.8/resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala#L491-L503).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
