capistrant commented on code in PR #18302:
URL: https://github.com/apache/druid/pull/18302#discussion_r2255284939
##########
server/src/test/java/org/apache/druid/server/metrics/LatchableEmitter.java:
##########
@@ -253,7 +255,7 @@ public EventMatcher hasMetricName(String metricName)
/**
* Matches an event only if it has the given metric value.
Review Comment:
nit
```suggestion
* Matches an event only if it has a value equal to or greater than the
given metric value.
```
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/psql/PostgreSQLMetadataStoreTest.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.psql;
+
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.apache.druid.testing.embedded.EmbeddedRouter;
+import org.apache.druid.testing.embedded.indexing.IndexTaskTest;
+
+/**
+ * Same as {@link IndexTaskTest}, but using a MariaDB metadata store instead
of Derby.
Review Comment:
nit
```suggestion
* Same as {@link IndexTaskTest}, but using a PostgreSQL metadata store
instead of Derby.
```
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/psql/PostgreSQLMetadataResource.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.psql;
+
+import org.apache.druid.java.util.common.StringUtils;
+import
org.apache.druid.metadata.storage.postgresql.PostgreSQLMetadataStorageModule;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.apache.druid.testing.embedded.TestcontainerResource;
+import org.testcontainers.containers.PostgreSQLContainer;
+
+/**
+ * Resource that creates a MariaDB metadata store.
Review Comment:
nit
```suggestion
* Resource that creates a PostgreSQL metadata store.
```
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/docker/CustomNodeRoleDockerTest.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.docker;
+
+import org.apache.druid.testing.DruidCommand;
+import org.apache.druid.testing.cli.CliEventCollector;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.apache.druid.testing.embedded.EmbeddedRouter;
+import org.apache.druid.testing.embedded.server.HighAvailabilityTest;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+
+public class CustomNodeRoleDockerTest extends DockerTestBase
Review Comment:
nit: short javadoc relaying what we are testing and why it is valuable test
will be helpful for future folks checking in on this
##########
services/src/test/java/org/apache/druid/testing/embedded/EmbeddedDruidCluster.java:
##########
@@ -232,13 +228,21 @@ public TestFolder getTestFolder()
}
/**
- * The embedded Zookeeper server used by this cluster, if any.
- *
- * @throws NullPointerException if this cluster has no embedded zookeeper.
+ * Uses a container-friendly hostname for all embedded services, Druid as
well
+ * as external.
+ */
+ public EmbeddedDruidCluster useContainerFriendlyHostname()
Review Comment:
what is the reason of having clusters opt into containerFriendlyHostname? Is
there a downside or problem of using it if you don't have to, say for an
existing embedded test you have written, HighAvailability?
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/docker/IngestionBackwardCompatibilityDockerTest.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.docker;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.druid.client.coordinator.CoordinatorServiceClient;
+import org.apache.druid.java.util.common.StringUtils;
+import
org.apache.druid.java.util.http.client.response.BytesFullResponseHandler;
+import org.apache.druid.java.util.http.client.response.BytesFullResponseHolder;
+import org.apache.druid.rpc.RequestBuilder;
+import org.apache.druid.rpc.indexing.SegmentUpdateResponse;
+import org.apache.druid.testing.DruidContainer;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.jboss.netty.handler.codec.http.HttpMethod;
+import org.junit.jupiter.api.Nested;
+
+/**
+ * Runs some basic ingestion tests using {@code DruidContainers} to verify
+ * backward compatibility with old Druid images.
+ */
+public class IngestionBackwardCompatibilityDockerTest
+{
+ @Nested
+ public class Apache31 extends IngestionDockerTest
+ {
+ @Override
+ public EmbeddedDruidCluster createCluster()
+ {
+ coordinator.usingImage(DruidContainer.Image.APACHE_31);
Review Comment:
is this something someone is going to have to periodically update and commit
to master?
--
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]
