imbajin commented on code in PR #724:
URL:
https://github.com/apache/hugegraph-toolchain/pull/724#discussion_r3071747443
##########
hugegraph-loader/assembly/travis/install-mysql.sh:
##########
@@ -23,12 +23,20 @@ TRAVIS_DIR=$(dirname $0)
CONF=hugegraph-test/src/main/resources/hugegraph.properties
MYSQL_USERNAME=root
-# Set MySQL configurations
+DB_NAME="${1:-mysql}"
+DB_PASS="${2:-root}"
+
+# Skip if MySQL is already running (e.g., provided by GitHub Actions service)
+if docker ps | grep -q "mysql:5.7"; then
Review Comment:
⚠️ This broad container check changes the script semantics in a risky way.
If a developer already has any unrelated `mysql:5.7` container running, the
script now exits without ensuring the requested test container
name/password/port match what the loader tests expect.
That can turn a clean setup failure into a much harder-to-diagnose
auth/connection failure later. Could we scope the idempotency check to the
requested container instead?
```suggestion
if docker ps --format '{{.Names}}' | grep -qx "${DB_NAME}"; then
echo "MySQL container ${DB_NAME} is already running, skipping setup."
exit 0
fi
```
Also consider upgrading the mysql version to `8.x` (stable version) if
possible
--
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]
