imbajin commented on code in PR #785:
URL:
https://github.com/apache/hugegraph-toolchain/pull/785#discussion_r4205261134
##########
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" == *#* ]]; then
+ echo "Cluster --file must be a local mapping path without # fragments"
>&2
+ return 2
+ fi
+ HUGEGRAPH_ARGS+=(--file "${file##*/}")
+ if [[ -z "$files" ]]; then
+ files=$file
+ else
+ files+=",$file"
Review Comment:
🧹 Blocking: no. Summary: A legal mapping path containing a comma is folded
into Spark's comma-delimited --files list and parsed as multiple resources.
Please reject comma-containing cluster mapping paths with a clear error or
encode them in a Spark-supported way. Evidence: exact-head
get-params.sh:141-155; Apache Spark 3.5.8 defines --files as a comma-separated
list:
https://github.com/apache/spark/blob/v3.5.8/core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala#L2397-L2401.
##########
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}
Review Comment:
⚠️ Blocking: no. Summary: The --conf form is parsed for mode selection, but
--properties-file is only forwarded; when that file selects cluster mode, mode
remains empty here and the mapping is passed as a submit-host --file path
instead of being staged in --files. Please resolve this supported config source
and Spark defaults before routing, or reject cluster mode when the effective
mode cannot be determined. Evidence: exact-head get-params.sh:1192-1214,
133-155; Spark 3.5.8 loads properties-file settings and uses them to resolve
deployMode:
https://github.com/apache/spark/blob/v3.5.8/core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala.
--
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]
