You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "ptupitsyn (via GitHub)" <gi...@apache.org> on 2025/06/02 05:17:35 UTC
[PR] IGNITE-24462 Add Java client protocol compatibility tests - current client with old servers [ignite-3]
ptupitsyn opened a new pull request, #5950:
URL: https://github.com/apache/ignite-3/pull/5950
(no comment)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-24462 Add Java client protocol compatibility tests - current client with old servers [ignite-3]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5950:
URL: https://github.com/apache/ignite-3/pull/5950#discussion_r2120075319
##########
modules/client/src/integrationTest/java/org/apache/ignite/client/compatibility/containers/IgniteServerContainer.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.ignite.client.compatibility.containers;
+
+import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.output.OutputFrame;
+import org.testcontainers.containers.wait.strategy.Wait;
+import org.testcontainers.lifecycle.Startable;
+import org.testcontainers.lifecycle.Startables;
+import org.testcontainers.utility.DockerImageName;
+
+/**
+ * Container for an Ignite server instance.
+ * This container uses the Apache Ignite 3.0.0 image.
+ */
+public class IgniteServerContainer implements Startable {
+ private static final DockerImageName IGNITE_IMAGE = DockerImageName.parse("apacheignite/ignite");
+
+ private final GenericContainer<?> container;
+
+ public IgniteServerContainer(String tag) {
+ this.container = createIgniteContainer(tag);
+ }
+
+ @SuppressWarnings("resource")
+ private static GenericContainer<?> createIgniteContainer(String tag) {
+ Consumer<OutputFrame> logConsumer = outputFrame -> System.out.print(outputFrame.getUtf8String());
+
+ return new GenericContainer<>(IGNITE_IMAGE.withTag(tag))
+ .withExposedPorts(10300, 10800)
+ .withLogConsumer(logConsumer)
+ .waitingFor(Wait.forLogMessage(".*Joining the cluster.*?", 1));
Review Comment:
[nitpick] The regex uses a reluctant quantifier (".*?"), which may not match the full log message as intended; consider using ".*Joining the cluster.*" for a clearer, more reliable pattern.
```suggestion
.waitingFor(Wait.forLogMessage(".*Joining the cluster.*", 1));
```
##########
modules/client/src/integrationTest/java/org/apache/ignite/client/compatibility/AbstractCurrentClientWithOldServerCompatibilityTest.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.ignite.client.compatibility;
+
+import static org.apache.ignite.internal.util.IgniteUtils.closeAll;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import org.apache.ignite.client.IgniteClient;
+import org.apache.ignite.client.compatibility.containers.IgniteServerContainer;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+/**
+ * Tests that current Java client can work with all older server versions.
+ */
+public abstract class AbstractCurrentClientWithOldServerCompatibilityTest extends ClientCompatibilityTestBase {
+ private IgniteServerContainer serverContainer;
+
+ abstract String serverVersion();
+
+ @BeforeAll
+ @Override
+ public void beforeAll() throws Exception {
+ serverContainer = new IgniteServerContainer(serverVersion());
+ serverContainer.start();
+
+ activateCluster(serverContainer.restPort());
+ waitForActivation(serverContainer.clientPort());
+
+ client = IgniteClient.builder()
+ .addresses("localhost:" + serverContainer.clientPort())
+ .build();
+
+ super.beforeAll();
+ }
+
+ @AfterAll
+ void afterAll() throws Exception {
+ closeAll(client, serverContainer);
+ }
+
+ private static void activateCluster(int restPort) throws IOException {
+ URL url = new URL("http://localhost:" + restPort + "/management/v1/cluster/init");
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ conn.setRequestMethod("POST");
+ conn.setRequestProperty("Content-Type", "application/json");
+ conn.setDoOutput(true);
+
+ String jsonInput = "{\"metaStorageNodes\": [\"defaultNode\"], \"clusterName\": \"myCluster\"}";
+
+ try (OutputStream os = conn.getOutputStream()) {
+ os.write(jsonInput.getBytes());
+ os.flush();
+ }
+
+ int responseCode = conn.getResponseCode();
Review Comment:
The HTTP activation response code is obtained but not validated; consider checking for a successful (200) status or throwing an exception to fail fast if activation fails.
```suggestion
int responseCode = conn.getResponseCode();
if (responseCode != HttpURLConnection.HTTP_OK) {
throw new IOException("Failed to activate cluster. HTTP response code: " + responseCode);
}
```
##########
modules/client/src/integrationTest/java/org/apache/ignite/client/compatibility/AbstractCurrentClientWithOldServerCompatibilityTest.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.ignite.client.compatibility;
+
+import static org.apache.ignite.internal.util.IgniteUtils.closeAll;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import org.apache.ignite.client.IgniteClient;
+import org.apache.ignite.client.compatibility.containers.IgniteServerContainer;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+/**
+ * Tests that current Java client can work with all older server versions.
+ */
+public abstract class AbstractCurrentClientWithOldServerCompatibilityTest extends ClientCompatibilityTestBase {
+ private IgniteServerContainer serverContainer;
+
+ abstract String serverVersion();
+
+ @BeforeAll
+ @Override
+ public void beforeAll() throws Exception {
+ serverContainer = new IgniteServerContainer(serverVersion());
+ serverContainer.start();
+
+ activateCluster(serverContainer.restPort());
+ waitForActivation(serverContainer.clientPort());
+
+ client = IgniteClient.builder()
+ .addresses("localhost:" + serverContainer.clientPort())
+ .build();
+
+ super.beforeAll();
+ }
+
+ @AfterAll
+ void afterAll() throws Exception {
+ closeAll(client, serverContainer);
+ }
+
+ private static void activateCluster(int restPort) throws IOException {
+ URL url = new URL("http://localhost:" + restPort + "/management/v1/cluster/init");
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ conn.setRequestMethod("POST");
+ conn.setRequestProperty("Content-Type", "application/json");
+ conn.setDoOutput(true);
+
+ String jsonInput = "{\"metaStorageNodes\": [\"defaultNode\"], \"clusterName\": \"myCluster\"}";
+
+ try (OutputStream os = conn.getOutputStream()) {
+ os.write(jsonInput.getBytes());
+ os.flush();
+ }
+
+ int responseCode = conn.getResponseCode();
+ System.out.println("Response Code: " + responseCode);
Review Comment:
[nitpick] Using System.out.println for logging in test setup may lead to inconsistent output; consider using a logger for structured and configurable logging.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-24462 Add Java client protocol compatibility tests - current client with old servers [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn merged PR #5950:
URL: https://github.com/apache/ignite-3/pull/5950
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org