Copilot commented on code in PR #11909:
URL: https://github.com/apache/gravitino/pull/11909#discussion_r3534442089
##########
integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TrinoITContainers.java:
##########
@@ -106,78 +115,51 @@ private void resolveServerAddress() throws Exception {
String command = ITUtils.joinPath(dockerComposeDir, "inspect_ip.sh");
Object output =
CommandExecutor.executeCommandLocalHost(
- command, false, ProcessData.TypesOfData.STREAMS_MERGED);
+ command, false, ProcessData.TypesOfData.STREAMS_MERGED,
containerEnv);
LOG.info("Command {} output:\n{}", command, output);
// expect the output to be like:
- // trino:10.20.30.21
- // hive:10.20.30.19
- // mysql:10.20.30.20
- // postgresql:10.20.30.18
-
- String containerIpMapping = output.toString();
- if (containerIpMapping.isEmpty()) {
+ // trino_uri=http://10.20.30.21:8080
+ // hive_uri=thrift://10.20.30.19:9083
+ // hdfs_uri=hdfs://10.20.30.19:9000
+ // mysql_uri=jdbc:mysql://10.20.30.20:3306
+ // postgresql_uri=jdbc:postgresql://10.20.30.18
+
+ String serviceURLs = output.toString();
+ if (serviceURLs.isEmpty()) {
throw new ContainerLaunchException(
"Failed to get the container status, the containers have not
started");
}
try {
- String[] containerInfos = containerIpMapping.split("\n");
- for (String container : containerInfos) {
- String[] info = container.split(":");
-
- String containerName = info[0];
- String address = info[1];
-
- if (containerName.equals("trino")) {
- servicesUri.put("trino", String.format("http://%s:8080";, address));
- } else if (containerName.equals("hive")) {
- servicesUri.put("hive_metastore", String.format("thrift://%s:9083",
address));
- servicesUri.put("hdfs", String.format("hdfs://%s:9000", address));
- } else if (containerName.equals("mysql")) {
- servicesUri.put("mysql", String.format("jdbc:mysql://%s:3306",
address));
- } else if (containerName.equals("postgresql")) {
- servicesUri.put("postgresql", String.format("jdbc:postgresql://%s",
address));
+ String[] serviceInfos = serviceURLs.split("\n");
+ for (String serviceInfo : serviceInfos) {
+ String[] info = serviceInfo.split("=", 2);
+ if (info.length != 2) {
+ continue;
}
+
+ servicesUri.put(info[0], info[1]);
}
} catch (Exception e) {
- throw new ContainerLaunchException("Unexpected container status :\n" +
containerIpMapping, e);
+ throw new ContainerLaunchException("Unexpected container status :\n" +
serviceURLs, e);
}
- for (String serviceName : servicesName) {
- if (!servicesUri.containsKey(serviceName)) {
- throw new ContainerLaunchException(
- String.format("The container for the %s service is not started: ",
serviceName));
- }
+ if (!servicesUri.containsKey("trino_uri")) {
+ throw new ContainerLaunchException("The container for the trino service
is not started");
}
}
public void shutdown() {
String command = ITUtils.joinPath(dockerComposeDir, "shutdown.sh");
Object output =
CommandExecutor.executeCommandLocalHost(
- command, false, ProcessData.TypesOfData.STREAMS_MERGED);
+ command, false, ProcessData.TypesOfData.STREAMS_MERGED,
containerEnv);
LOG.info("Command {} output:\n{}", command, output);
}
- public String getTrinoUri() {
- return servicesUri.get("trino");
- }
-
- public String getHiveMetastoreUri() {
- return servicesUri.get("hive_metastore");
- }
-
- public String getHdfsUri() {
- return servicesUri.get("hdfs");
- }
-
- public String getMysqlUri() {
- return servicesUri.get("mysql");
- }
-
- public String getPostgresqlUri() {
- return servicesUri.get("postgresql");
+ public Map<String, String> getServiceUrls() {
+ return servicesUri;
}
Review Comment:
getServiceUrls() currently returns the internal mutable servicesUri map.
Callers can accidentally mutate it (or observe mutations across launches),
which can lead to flaky IT behavior. Return an unmodifiable snapshot instead.
##########
integration-test-common/docker-script/shutdown.sh:
##########
@@ -22,10 +22,14 @@ cd "$(dirname "$0")"
LOG_DIR=../build/trino-ci-container-log
if [ -d $LOG_DIR ]; then
- docker cp trino-ci-hive:/usr/local/hadoop/logs $LOG_DIR/hdfs
- docker cp trino-ci-hive:/tmp/root $LOG_DIR/hive
- docker cp trino-ci-trino:/tmp/trino.log $LOG_DIR/trino.log
+ if [ -n "$(docker ps -aq -f name=trino-ci-hive)" ]; then
+ docker cp trino-ci-hive:/usr/local/hadoop/logs $LOG_DIR/hdfs
+ docker cp trino-ci-hive:/tmp/root $LOG_DIR/hive
+ fi
+ if [ -n "$(docker ps -aq -f name=trino-ci-trino)" ]; then
+ docker cp trino-ci-trino:/tmp/trino.log $LOG_DIR/trino.log
+ fi
fi
export GRAVITINO_TRINO_CONNECTOR_DIR=/dev/null
-docker compose down
+docker compose down --remove-orphans
Review Comment:
shutdown.sh always runs `docker compose down` without selecting the compose
file. When TEST_CONTAINERS selects a subset compose file, shutdown may target
the wrong config (and will break if future subsets define services not present
in docker-compose.yaml). Source compose-env.sh and pass `-f "$COMPOSE_FILE"` to
keep shutdown consistent with launch.sh.
--
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]
