jerryshao commented on code in PR #10377:
URL: https://github.com/apache/gravitino/pull/10377#discussion_r2930379499


##########
dev/docker/gravitino/start-gravitino.sh:
##########
@@ -26,6 +26,38 @@ cd ${gravitino_dir}
 
 python bin/rewrite_gravitino_server_config.py
 
+# Create soft links for JDBC drivers
+jdbc_driver_dir="${gravitino_dir}/jdbc-drivers"
+
+if [ -d "${jdbc_driver_dir}" ]; then
+  # Link MySQL driver to catalogs that need it
+  if [ -f "${jdbc_driver_dir}/mysql-connector-java-8.0.27.jar" ]; then
+    ln -sv "${jdbc_driver_dir}/mysql-connector-java-8.0.27.jar" 
"${gravitino_dir}/catalogs/jdbc-mysql/libs/"

Review Comment:
   Hard-coded driver filename will silently skip linking if the version is ever 
bumped in `gravitino-dependency.sh`. The `if [ -f ... ]` guard will be false 
and the JDBC drivers will not be linked, breaking catalog connectivity without 
any error message.
   
   The iceberg bundle section in this same PR avoids this by using `find ... 
-name "*.jar"`. Consider the same pattern here:
   ```sh
   find "${jdbc_driver_dir}" -name "mysql-connector-java-*.jar" -exec ln -sfv 
{} "${gravitino_dir}/catalogs/jdbc-mysql/libs/" \;
   ```
   Reference: similar version-coupling breakage in 
[#9874](https://github.com/apache/gravitino/pull/9874).



##########
dev/docker/gravitino/start-gravitino.sh:
##########
@@ -26,6 +26,38 @@ cd ${gravitino_dir}
 
 python bin/rewrite_gravitino_server_config.py
 
+# Create soft links for JDBC drivers
+jdbc_driver_dir="${gravitino_dir}/jdbc-drivers"
+
+if [ -d "${jdbc_driver_dir}" ]; then
+  # Link MySQL driver to catalogs that need it
+  if [ -f "${jdbc_driver_dir}/mysql-connector-java-8.0.27.jar" ]; then
+    ln -sv "${jdbc_driver_dir}/mysql-connector-java-8.0.27.jar" 
"${gravitino_dir}/catalogs/jdbc-mysql/libs/"
+    ln -sv "${jdbc_driver_dir}/mysql-connector-java-8.0.27.jar" 
"${gravitino_dir}/catalogs/jdbc-doris/libs/"

Review Comment:
   If any target `libs/` directory does not exist (e.g. a catalog is removed 
from the distribution), `ln -sv` will fail and `set -e` (line 21) will abort 
the container before Gravitino starts. Consider adding `mkdir -p` guards before 
each `ln` block, or checking that the target directory exists first.
   
   This exact failure mode was hit in 
[#9874](https://github.com/apache/gravitino/pull/9874) when 
`catalogs/jdbc-oceanbase/libs/` was absent.



-- 
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]

Reply via email to