Copilot commented on code in PR #60061:
URL: https://github.com/apache/doris/pull/60061#discussion_r2707962401
##########
run-regression-test.sh:
##########
@@ -172,30 +172,81 @@ fi
if ! test -f ${RUN_JAR:+${RUN_JAR}}; then
echo "===== Build Regression Test Framework ====="
- # echo "Build generated code"
- cd "${DORIS_HOME}/gensrc/thrift"
- make
+ # Configure Maven for better network resilience
+ export MAVEN_OPTS="${MAVEN_OPTS:-} -Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.httpconnectionManager.ttlSeconds=25
-Dmaven.wagon.http.retryHandler.class=standard"
+
+ # Function to execute Maven command with retry logic
+ execute_maven_with_retry() {
+ local cmd="$1"
+ local max_attempts=3
+ local attempt=1
+
+ while [[ ${attempt} -le ${max_attempts} ]]; do
+ echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
+ if eval "${cmd}"; then
Review Comment:
Using eval with user-controllable variables (cmd parameter) can be a
security risk if the MVN_CMD variable can be influenced by external input.
Consider using a safer alternative like directly invoking the command or using
an array to store command components. If eval is necessary, ensure MVN_CMD is
validated or comes from a trusted source only.
```suggestion
local -a cmd_array
# Split the command string into an array of arguments and execute it
directly
read -r -a cmd_array <<< "${cmd}"
while [[ ${attempt} -le ${max_attempts} ]]; do
echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
if "${cmd_array[@]}"; then
```
##########
run-regression-test.sh:
##########
@@ -172,30 +172,81 @@ fi
if ! test -f ${RUN_JAR:+${RUN_JAR}}; then
echo "===== Build Regression Test Framework ====="
- # echo "Build generated code"
- cd "${DORIS_HOME}/gensrc/thrift"
- make
+ # Configure Maven for better network resilience
+ export MAVEN_OPTS="${MAVEN_OPTS:-} -Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.httpconnectionManager.ttlSeconds=25
-Dmaven.wagon.http.retryHandler.class=standard"
+
+ # Function to execute Maven command with retry logic
+ execute_maven_with_retry() {
+ local cmd="$1"
+ local max_attempts=3
+ local attempt=1
+
+ while [[ ${attempt} -le ${max_attempts} ]]; do
+ echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
+ if eval "${cmd}"; then
+ echo "Command succeeded on attempt ${attempt}"
+ return 0
+ else
+ echo "Command failed on attempt ${attempt}"
+ if [[ ${attempt} -lt ${max_attempts} ]]; then
+ sleep $((attempt * 5)) # Exponential backoff
Review Comment:
The backoff calculation uses linear backoff (attempt * 5 seconds), not
exponential backoff as the comment states. For exponential backoff, the formula
should be something like 2^attempt or 5 * 2^(attempt-1). Consider either
correcting the comment to say "linear backoff" or implementing actual
exponential backoff.
```suggestion
sleep $((attempt * 5)) # Linear backoff
```
##########
run-regression-test.sh:
##########
@@ -172,30 +172,81 @@ fi
if ! test -f ${RUN_JAR:+${RUN_JAR}}; then
echo "===== Build Regression Test Framework ====="
- # echo "Build generated code"
- cd "${DORIS_HOME}/gensrc/thrift"
- make
+ # Configure Maven for better network resilience
+ export MAVEN_OPTS="${MAVEN_OPTS:-} -Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.httpconnectionManager.ttlSeconds=25
-Dmaven.wagon.http.retryHandler.class=standard"
+
+ # Function to execute Maven command with retry logic
+ execute_maven_with_retry() {
+ local cmd="$1"
+ local max_attempts=3
+ local attempt=1
+
+ while [[ ${attempt} -le ${max_attempts} ]]; do
+ echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
+ if eval "${cmd}"; then
+ echo "Command succeeded on attempt ${attempt}"
+ return 0
+ else
+ echo "Command failed on attempt ${attempt}"
+ if [[ ${attempt} -lt ${max_attempts} ]]; then
+ sleep $((attempt * 5)) # Exponential backoff
+ fi
+ ((attempt++))
+ fi
+ done
+
+ echo "Command failed after ${max_attempts} attempts"
+ return 1
+ }
+
+ # Build generated code
+ cd "${DORIS_HOME}/gensrc/thrift" || { echo "Failed to change directory";
exit 1; }
+ if ! make; then
+ echo "Make command failed in ${DORIS_HOME}/gensrc/thrift"
+ exit 1
+ fi
cp -rf "${DORIS_HOME}/gensrc/build/gen_java/org/apache/doris/thrift"
"${FRAMEWORK_APACHE_DIR}/doris/"
- cd "${DORIS_HOME}/regression-test/framework"
- "${MVN_CMD}" package
- cd "${DORIS_HOME}"
+ # Navigate to framework directory and build with retry
+ cd "${DORIS_HOME}/regression-test/framework" || { echo "Failed to change
directory"; exit 1; }
+
+ # First try to download dependencies only
+ echo "Downloading dependencies..."
+ execute_maven_with_retry "\"${MVN_CMD}\" dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=/tmp/dependencies.txt"
|| {
Review Comment:
The dependency resolution step writes output to /tmp/dependencies.txt which
may cause issues in multi-user environments or concurrent builds due to
potential file conflicts. Consider using a unique temporary filename or writing
to a location within the build directory instead.
```suggestion
dep_output_file="$(mktemp -t doris-dependencies-XXXXXX.txt)" || { echo
"Failed to create temporary file for dependency output"; exit 1; }
execute_maven_with_retry "\"${MVN_CMD}\" dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=${dep_output_file}" ||
{
```
##########
run-regression-test.sh:
##########
@@ -172,30 +172,81 @@ fi
if ! test -f ${RUN_JAR:+${RUN_JAR}}; then
echo "===== Build Regression Test Framework ====="
- # echo "Build generated code"
- cd "${DORIS_HOME}/gensrc/thrift"
- make
+ # Configure Maven for better network resilience
+ export MAVEN_OPTS="${MAVEN_OPTS:-} -Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.httpconnectionManager.ttlSeconds=25
-Dmaven.wagon.http.retryHandler.class=standard"
+
+ # Function to execute Maven command with retry logic
+ execute_maven_with_retry() {
+ local cmd="$1"
+ local max_attempts=3
+ local attempt=1
+
+ while [[ ${attempt} -le ${max_attempts} ]]; do
+ echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
+ if eval "${cmd}"; then
+ echo "Command succeeded on attempt ${attempt}"
+ return 0
+ else
+ echo "Command failed on attempt ${attempt}"
+ if [[ ${attempt} -lt ${max_attempts} ]]; then
+ sleep $((attempt * 5)) # Exponential backoff
+ fi
+ ((attempt++))
+ fi
+ done
+
+ echo "Command failed after ${max_attempts} attempts"
+ return 1
+ }
+
+ # Build generated code
+ cd "${DORIS_HOME}/gensrc/thrift" || { echo "Failed to change directory";
exit 1; }
+ if ! make; then
+ echo "Make command failed in ${DORIS_HOME}/gensrc/thrift"
+ exit 1
+ fi
cp -rf "${DORIS_HOME}/gensrc/build/gen_java/org/apache/doris/thrift"
"${FRAMEWORK_APACHE_DIR}/doris/"
- cd "${DORIS_HOME}/regression-test/framework"
- "${MVN_CMD}" package
- cd "${DORIS_HOME}"
+ # Navigate to framework directory and build with retry
+ cd "${DORIS_HOME}/regression-test/framework" || { echo "Failed to change
directory"; exit 1; }
+
+ # First try to download dependencies only
+ echo "Downloading dependencies..."
+ execute_maven_with_retry "\"${MVN_CMD}\" dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=/tmp/dependencies.txt"
|| {
+ echo "Failed to download dependencies"
+ exit 1
+ }
+
+ # Then package with retry
+ echo "Building package..."
+ execute_maven_with_retry "\"${MVN_CMD}\" clean package -B -DskipTests=true
-Dmaven.javadoc.skip=true" || {
Review Comment:
The quoting around MVN_CMD in the Maven commands passed to
execute_maven_with_retry will cause issues. The escaped quotes \"${MVN_CMD}\"
will be passed literally to eval, resulting in extra quotes around the Maven
command when it's executed. This should be changed to remove the escaped quotes
around MVN_CMD, as eval will properly handle the variable expansion.
```suggestion
execute_maven_with_retry "${MVN_CMD} dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=/tmp/dependencies.txt"
|| {
echo "Failed to download dependencies"
exit 1
}
# Then package with retry
echo "Building package..."
execute_maven_with_retry "${MVN_CMD} clean package -B -DskipTests=true
-Dmaven.javadoc.skip=true" || {
```
##########
run-regression-test.sh:
##########
@@ -172,30 +172,81 @@ fi
if ! test -f ${RUN_JAR:+${RUN_JAR}}; then
echo "===== Build Regression Test Framework ====="
- # echo "Build generated code"
- cd "${DORIS_HOME}/gensrc/thrift"
- make
+ # Configure Maven for better network resilience
+ export MAVEN_OPTS="${MAVEN_OPTS:-} -Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.httpconnectionManager.ttlSeconds=25
-Dmaven.wagon.http.retryHandler.class=standard"
+
+ # Function to execute Maven command with retry logic
+ execute_maven_with_retry() {
+ local cmd="$1"
+ local max_attempts=3
+ local attempt=1
+
+ while [[ ${attempt} -le ${max_attempts} ]]; do
+ echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
+ if eval "${cmd}"; then
+ echo "Command succeeded on attempt ${attempt}"
+ return 0
+ else
+ echo "Command failed on attempt ${attempt}"
+ if [[ ${attempt} -lt ${max_attempts} ]]; then
+ sleep $((attempt * 5)) # Exponential backoff
+ fi
+ ((attempt++))
+ fi
+ done
+
+ echo "Command failed after ${max_attempts} attempts"
+ return 1
+ }
+
+ # Build generated code
+ cd "${DORIS_HOME}/gensrc/thrift" || { echo "Failed to change directory";
exit 1; }
+ if ! make; then
+ echo "Make command failed in ${DORIS_HOME}/gensrc/thrift"
+ exit 1
+ fi
cp -rf "${DORIS_HOME}/gensrc/build/gen_java/org/apache/doris/thrift"
"${FRAMEWORK_APACHE_DIR}/doris/"
- cd "${DORIS_HOME}/regression-test/framework"
- "${MVN_CMD}" package
- cd "${DORIS_HOME}"
+ # Navigate to framework directory and build with retry
+ cd "${DORIS_HOME}/regression-test/framework" || { echo "Failed to change
directory"; exit 1; }
+
+ # First try to download dependencies only
+ echo "Downloading dependencies..."
+ execute_maven_with_retry "\"${MVN_CMD}\" dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=/tmp/dependencies.txt"
|| {
+ echo "Failed to download dependencies"
+ exit 1
+ }
+
+ # Then package with retry
+ echo "Building package..."
+ execute_maven_with_retry "\"${MVN_CMD}\" clean package -B -DskipTests=true
-Dmaven.javadoc.skip=true" || {
Review Comment:
The quoting around MVN_CMD in the Maven commands passed to
execute_maven_with_retry will cause issues. The escaped quotes \"${MVN_CMD}\"
will be passed literally to eval, resulting in extra quotes around the Maven
command when it's executed. This should be changed to remove the escaped quotes
around MVN_CMD, as eval will properly handle the variable expansion.
```suggestion
execute_maven_with_retry "${MVN_CMD} dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=/tmp/dependencies.txt"
|| {
echo "Failed to download dependencies"
exit 1
}
# Then package with retry
echo "Building package..."
execute_maven_with_retry "${MVN_CMD} clean package -B -DskipTests=true
-Dmaven.javadoc.skip=true" || {
```
##########
run-regression-test.sh:
##########
@@ -172,30 +172,81 @@ fi
if ! test -f ${RUN_JAR:+${RUN_JAR}}; then
echo "===== Build Regression Test Framework ====="
- # echo "Build generated code"
- cd "${DORIS_HOME}/gensrc/thrift"
- make
+ # Configure Maven for better network resilience
+ export MAVEN_OPTS="${MAVEN_OPTS:-} -Dmaven.wagon.http.retryHandler.count=3
-Dmaven.wagon.httpconnectionManager.ttlSeconds=25
-Dmaven.wagon.http.retryHandler.class=standard"
+
+ # Function to execute Maven command with retry logic
+ execute_maven_with_retry() {
+ local cmd="$1"
+ local max_attempts=3
+ local attempt=1
+
+ while [[ ${attempt} -le ${max_attempts} ]]; do
+ echo "Attempt ${attempt}/${max_attempts}: ${cmd}"
+ if eval "${cmd}"; then
+ echo "Command succeeded on attempt ${attempt}"
+ return 0
+ else
+ echo "Command failed on attempt ${attempt}"
+ if [[ ${attempt} -lt ${max_attempts} ]]; then
+ sleep $((attempt * 5)) # Exponential backoff
+ fi
+ ((attempt++))
+ fi
+ done
+
+ echo "Command failed after ${max_attempts} attempts"
+ return 1
+ }
+
+ # Build generated code
+ cd "${DORIS_HOME}/gensrc/thrift" || { echo "Failed to change directory";
exit 1; }
+ if ! make; then
+ echo "Make command failed in ${DORIS_HOME}/gensrc/thrift"
+ exit 1
+ fi
cp -rf "${DORIS_HOME}/gensrc/build/gen_java/org/apache/doris/thrift"
"${FRAMEWORK_APACHE_DIR}/doris/"
- cd "${DORIS_HOME}/regression-test/framework"
- "${MVN_CMD}" package
- cd "${DORIS_HOME}"
+ # Navigate to framework directory and build with retry
+ cd "${DORIS_HOME}/regression-test/framework" || { echo "Failed to change
directory"; exit 1; }
+
+ # First try to download dependencies only
+ echo "Downloading dependencies..."
+ execute_maven_with_retry "\"${MVN_CMD}\" dependency:resolve -B
-DskipTests=true -Dmdep.prependGroupId=true -DoutputFile=/tmp/dependencies.txt"
|| {
+ echo "Failed to download dependencies"
+ exit 1
+ }
+
+ # Then package with retry
+ echo "Building package..."
+ execute_maven_with_retry "\"${MVN_CMD}\" clean package -B -DskipTests=true
-Dmaven.javadoc.skip=true" || {
+ echo "Failed to build package"
+ exit 1
+ }
+
+ cd "${DORIS_HOME}" || { echo "Failed to return to DORIS_HOME"; exit 1; }
mkdir -p "${OUTPUT_DIR}"/{lib,log}
cp -r "${REGRESSION_TEST_BUILD_DIR}"/regression-test-*.jar
"${OUTPUT_DIR}/lib"
echo "===== BUILD JAVA_UDF_SRC TO GENERATE JAR ====="
mkdir -p "${DORIS_HOME}"/regression-test/suites/javaudf_p0/jars
- cd "${DORIS_HOME}"/regression-test/java-udf-src
- "${MVN_CMD}" package
+ cd "${DORIS_HOME}"/regression-test/java-udf-src || { echo "Failed to
change directory to java-udf-src"; exit 1; }
+
+ # Build UDF with retry
+ execute_maven_with_retry "\"${MVN_CMD}\" clean package -B -DskipTests=true
-Dmaven.javadoc.skip=true" || {
Review Comment:
The quoting around MVN_CMD in the Maven commands passed to
execute_maven_with_retry will cause issues. The escaped quotes \"${MVN_CMD}\"
will be passed literally to eval, resulting in extra quotes around the Maven
command when it's executed. This should be changed to remove the escaped quotes
around MVN_CMD, as eval will properly handle the variable expansion.
--
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]
