You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/02 15:45:10 UTC

[GitHub] [ignite] eadha opened a new pull request #8968: Ignite-13970-2

eadha opened a new pull request #8968:
URL: https://github.com/apache/ignite/pull/8968


   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] There is a single JIRA ticket related to the pull request. 
   - [ ] The web-link to the pull request is attached to the JIRA ticket.
   - [ ] The JIRA ticket has the _Patch Available_ state.
   - [ ] The pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
   - [ ] The pull request title is treated as the final commit message. 
   The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607604672



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -60,6 +61,11 @@ public static void main(String[] args) throws Exception {
 
         app.cfgPath = cfgPath;
 
+        String connStr = jsonNode.get("thin_client_connection").asText();
+
+        if (connStr != null && !connStr.isEmpty())

Review comment:
       Additional reflection suggests we may want to test the local Thin Client case, so, enum is not an option here.
   But, since `IgniteClient extends AutoCloseable` we must wrap the current try-with-resources section with an additional one.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607934174



##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -47,8 +47,9 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.java_class_name = java_class_name
         self.params = params
 
-    def await_started(self):
-        super().await_started()
+    def await_started(self, start_ignite=True):
+        if start_ignite:

Review comment:
       please, move this check to `ignite_aware.py`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r606939672



##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,70 @@
+# 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.
+
+"""
+This module contains client tests
+"""
+import time
+
+from ducktape.mark import parametrize
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size
+    CACHE_NAME - name of the cache to create for the test.
+    JAVA_CLIENT_CLASS_NAME - running classname.
+    static_clients - the number of permanently employed clients.
+    """
+
+    JAVA_CLIENT_CLASS_NAME = "org.apache.ignite.internal.ducktest.tests.thin_client_test.ThinClientSelfTest"
+
+    """
+    Tests services implementations
+    """
+
+    @cluster(num_nodes=2)
+    @ignite_versions(str(DEV_BRANCH))
+    @parametrize(num_nodes=2, clients_num=1)
+    def test_replicated_atomic_cache(self, ignite_version, num_nodes, clients_num):

Review comment:
       Let's remove `num_nodes` and `clients_num` and work with 2 nodes (1 server and 1 client) always.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] eadha closed pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
eadha closed pull request #8968:
URL: https://github.com/apache/ignite/pull/8968


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Sega76 commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
Sega76 commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607108714



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -60,6 +61,11 @@ public static void main(String[] args) throws Exception {
 
         app.cfgPath = cfgPath;
 
+        String connStr = jsonNode.get("thin_client_connection").asText();

Review comment:
       use Optional, otherwise we will get an Exception in other tests
   `Exception in thread "main" java.lang.NullPointerException
   	at org.apache.ignite.internal.ducktest.utils.IgniteAwareApplicationService.main(IgniteAwareApplicationService.java:64)`
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r608583343



##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,65 @@
+# 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.
+
+"""
+This module contains client tests.
+"""
+from ducktape.mark import matrix
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.services.utils.ssl.client_connector_configuration import ClientConnectorConfiguration
+from ignitetest.utils import cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size.
+    JAVA_CLIENT_CLASS_NAME - running classname.
+    """
+
+    JAVA_CLIENT_CLASS_NAME = "org.apache.ignite.internal.ducktest.tests.thin_client_test.ThinClientSelfTestApplication"
+
+    @cluster(num_nodes=2)
+    @matrix(server_version=[str(DEV_BRANCH), str(LATEST)], thin_client_version=[str(DEV_BRANCH), str(LATEST)])
+    def test_thin_client_compatibility(self, server_version, thin_client_version):
+        """
+        Thin client compatibility test.
+        """
+
+        server_config = IgniteConfiguration(version=IgniteVersion(server_version),
+                                            client_connector_configuration=ClientConnectorConfiguration())
+
+        ignite = IgniteService(self.test_context, server_config, 1)
+
+        thin_client_connection = ignite.nodes[0].account.hostname + ":" + str(server_config.
+                                                                              client_connector_configuration.port)
+
+        thin_clients = IgniteApplicationService(self.test_context,
+                                                IgniteConfiguration(version=IgniteVersion(thin_client_version)),
+                                                java_class_name=self.JAVA_CLIENT_CLASS_NAME,
+                                                num_nodes=1,
+                                                params={"thin_client_connection": thin_client_connection},
+                                                start_ignite=False)
+
+        ignite.start()
+        thin_clients.run()
+
+        thin_clients.await_event(evt_message="IGNITE_APPLICATION_FINISHED", timeout_sec=60, from_the_beginning=True)
+        thin_clients.stop()

Review comment:
       All we need is just to call service.run()
   It will start, wait and stop the service.
   Also, it will checks statuses for each state (eg. IGNITE_APPLICATION_FINISHED will be checked on stop phase)




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r608529537



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -76,7 +81,18 @@ public static void main(String[] args) throws Exception {
                 log.info("Ignite instance closed. [interrupted=" + Thread.currentThread().isInterrupted() + "]");
             }
         }
-        else
-            app.start(jsonNode);
+        else {
+            String connStr = jsonNode.get(THIN_CLIENT_CONNECTION).asText();
+
+            try (IgniteClient client = Ignition.startClient(new ClientConfiguration().setAddresses(connStr))) {
+                app.client = client;
+
+                app.start(jsonNode);
+            }
+            finally {
+                log.info("Thin client instance closed. [interrupted=" + Thread.currentThread().isInterrupted() + "]");
+            }
+

Review comment:
       please, remove empty line.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607109619



##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -46,14 +46,17 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.servicejava_class_name = servicejava_class_name
         self.java_class_name = java_class_name
         self.params = params
+        self.start_ignite = start_ignite

Review comment:
       An incorrect place to store this flag.
   The flag ... de-facto ... is never used inside the class.
   Should be stored at IgniteAwareService.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] eadha commented on pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
eadha commented on pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#issuecomment-812586411


   Not correct target


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607092674



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/thin_client_test/ThinClientSelfTest.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.ignite.internal.ducktest.tests.thin_client_test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.client.ClientCache;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+
+/**
+ * Thin client. Cache test: put, get, remove.
+ */
+public class ThinClientSelfTest extends IgniteAwareApplication {
+    /** {@inheritDoc} */
+    @Override protected void run(JsonNode jsonNode) throws Exception {
+        markInitialized();
+
+        ClientCache<Integer, Integer> cache = client.getOrCreateCache("testCache");
+
+        cache.put(0, 0);
+
+        Integer val = cache.get(0);

Review comment:
       variable newer used.

##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -46,14 +46,17 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.servicejava_class_name = servicejava_class_name
         self.java_class_name = java_class_name
         self.params = params
+        self.start_ignite = start_ignite
 
     def await_started(self):
-        super().await_started()
+        if self.start_ignite:
+            super().await_started()
 
         self.__check_status(self.APP_INIT_EVT_MSG, timeout=self.startup_timeout_sec)
 
     def await_stopped(self):
-        super().await_stopped()
+        if self.start_ignite:

Review comment:
       Incorrect usage.
   We still MUST wait for the Application to finish.

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_configuration/__init__.py
##########
@@ -45,6 +46,7 @@ class IgniteConfiguration(NamedTuple):
     local_host: str = None
     ssl_params: SslParams = None
     connector_configuration: ConnectorConfiguration = None
+    client_connector_configuration: ClientConnectorConfiguration = ClientConnectorConfiguration()

Review comment:
       Should this be enabled by explicit enabling?

##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -60,6 +61,11 @@ public static void main(String[] args) throws Exception {
 
         app.cfgPath = cfgPath;
 
+        String connStr = jsonNode.get("thin_client_connection").asText();
+
+        if (connStr != null && !connStr.isEmpty())

Review comment:
       We should have a proper switch here.
   `startIgnite` should be replaced with Enum [Node, ThinClient, None]
   The second param in this case should contain configuration for ThinClient if the `ThinClient` case selected.

##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -60,6 +61,11 @@ public static void main(String[] args) throws Exception {
 
         app.cfgPath = cfgPath;
 
+        String connStr = jsonNode.get("thin_client_connection").asText();

Review comment:
       Should be a constant at least. 
   But, see next comment why this not needed.

##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -46,14 +46,17 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.servicejava_class_name = servicejava_class_name
         self.java_class_name = java_class_name
         self.params = params
+        self.start_ignite = start_ignite

Review comment:
       An incorrect place to store this flag.
   The flag is never used inside the class.
   Should be stored at IgniteAwareService.

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/client_connector_configuration.py
##########
@@ -0,0 +1,30 @@
+# 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
+
+"""
+This module contains classes and utilities for Ignite ConnectorConfiguration.
+"""
+
+from typing import NamedTuple
+
+from ignitetest.services.utils.ssl.ssl_params import SslParams

Review comment:
       unused

##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,62 @@
+# 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.
+
+"""
+This module contains client tests.
+"""
+import time
+
+from ducktape.mark import parametrize, matrix
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size.
+    JAVA_CLIENT_CLASS_NAME - running classname.
+    """
+
+    JAVA_CLIENT_CLASS_NAME = "org.apache.ignite.internal.ducktest.tests.thin_client_test.ThinClientSelfTest"
+
+    @cluster(num_nodes=2)
+    @matrix(server_version=[str(DEV_BRANCH), str(LATEST)], thin_client_version=[str(DEV_BRANCH), str(LATEST)])
+    def test_thin_client_compatibility(self, server_version, thin_client_version):
+        """
+        Thin client compatibility test.
+        """
+
+        server_config = IgniteConfiguration(version=IgniteVersion(server_version))
+
+        ignite = IgniteService(self.test_context, server_config, 1)
+
+        thin_client_connection = ignite.nodes[0].account.hostname + ":" + str(server_config.client_connector_configuration.port)

Review comment:
       Use ThinClient config instead.

##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/thin_client_test/ThinClientSelfTest.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.ignite.internal.ducktest.tests.thin_client_test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.client.ClientCache;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+
+/**
+ * Thin client. Cache test: put, get, remove.
+ */
+public class ThinClientSelfTest extends IgniteAwareApplication {

Review comment:
       Should this be an Application since inherited from the Application?

##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/thin_client_test/ThinClientSelfTest.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.ignite.internal.ducktest.tests.thin_client_test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.client.ClientCache;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+
+/**
+ * Thin client. Cache test: put, get, remove.
+ */
+public class ThinClientSelfTest extends IgniteAwareApplication {
+    /** {@inheritDoc} */
+    @Override protected void run(JsonNode jsonNode) throws Exception {
+        markInitialized();
+
+        ClientCache<Integer, Integer> cache = client.getOrCreateCache("testCache");
+
+        cache.put(0, 0);
+
+        Integer val = cache.get(0);
+
+        cache.remove(0);
+
+        client.destroyCache("testCache");
+
+        markFinished();

Review comment:
       We should have at least one assertion here before finishing, eg. put == read.
   Otherwise, Volkswagen mode is 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607066584



##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,61 @@
+# 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.
+
+"""
+This module contains client tests.
+"""
+import time
+
+from ducktape.mark import parametrize, matrix
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size.
+    JAVA_CLIENT_CLASS_NAME - running classname.
+    """
+
+    JAVA_CLIENT_CLASS_NAME = "org.apache.ignite.internal.ducktest.tests.thin_client_test.ThinClientSelfTest"
+
+    @cluster(num_nodes=2)
+    @matrix(server_version=[str(DEV_BRANCH), str(LATEST)], thin_client_version=[str(DEV_BRANCH), str(LATEST)])
+    def test_thin_client_compatibility(self, server_version, thin_client_version):
+        """
+        Thin client compatibility test.
+        """
+
+        ignite = IgniteService(self.test_context, IgniteConfiguration(version=IgniteVersion(server_version), caches=[]), 1)
+
+        client_config = IgniteConfiguration(version=IgniteVersion(thin_client_version), caches=[])
+
+        thin_client_connection = ignite.nodes[0].account.hostname + ":" + str(client_config.client_connector_configuration.port)

Review comment:
       We should get port from *server* config.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r606939231



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/thin_client_test/ThinClientSelfTest.java
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.internal.ducktest.tests.thin_client_test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.client.ClientCache;
+import org.apache.ignite.client.ClientCacheConfiguration;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+
+/**
+ * Thin client. Cache test: put, get, value check

Review comment:
       dot in the end.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] eadha commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
eadha commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607135838



##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -46,14 +46,17 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.servicejava_class_name = servicejava_class_name
         self.java_class_name = java_class_name
         self.params = params
+        self.start_ignite = start_ignite

Review comment:
       it is used in await_started, string 52




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Sega76 commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
Sega76 commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607109273



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/thin_client_test/ThinClientSelfTest.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.ignite.internal.ducktest.tests.thin_client_test;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.client.ClientCache;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+
+/**
+ * Thin client. Cache test: put, get, remove.
+ */
+public class ThinClientSelfTest extends IgniteAwareApplication {
+    /** {@inheritDoc} */
+    @Override protected void run(JsonNode jsonNode) throws Exception {
+        markInitialized();
+
+        ClientCache<Integer, Integer> cache = client.getOrCreateCache("testCache");
+
+        cache.put(0, 0);
+
+        Integer val = cache.get(0);

Review comment:
       unused `Integer val`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov merged pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
nizhikov merged pull request #8968:
URL: https://github.com/apache/ignite/pull/8968


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607066096



##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -46,14 +46,17 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.servicejava_class_name = servicejava_class_name
         self.java_class_name = java_class_name
         self.params = params
+        self.start_ignite = start_ignite

Review comment:
       Why this change?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607920006



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -74,6 +74,7 @@ def __init__(self, context, config, num_nodes, startup_timeout_sec, shutdown_tim
         self.init_logs_attribute()
 
         self.disconnected_nodes = []
+        self.start_ignite = kwargs.get("start_ignite")

Review comment:
       you should provide the default value for `start_ignite` - `kwargs.get("start_ignite", True)`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r606940505



##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,70 @@
+# 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.
+
+"""
+This module contains client tests
+"""
+import time
+
+from ducktape.mark import parametrize
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size
+    CACHE_NAME - name of the cache to create for the test.
+    JAVA_CLIENT_CLASS_NAME - running classname.
+    static_clients - the number of permanently employed clients.
+    """
+
+    JAVA_CLIENT_CLASS_NAME = "org.apache.ignite.internal.ducktest.tests.thin_client_test.ThinClientSelfTest"
+
+    """
+    Tests services implementations
+    """
+
+    @cluster(num_nodes=2)
+    @ignite_versions(str(DEV_BRANCH))
+    @parametrize(num_nodes=2, clients_num=1)
+    def test_replicated_atomic_cache(self, ignite_version, num_nodes, clients_num):
+        """
+        Test that thin client can connect, create, configure  and use cache
+        """
+        servers_count = num_nodes - clients_num
+
+        server_configuration = IgniteConfiguration(version=IgniteVersion(ignite_version), caches=[])
+
+        ignite = IgniteService(self.test_context, server_configuration, servers_count, startup_timeout_sec=180)
+
+        thin_client_connection = ignite.nodes[0].account.hostname + ":10800"
+
+        static_clients = IgniteApplicationService(self.test_context, server_configuration,
+                                           java_class_name=self.JAVA_CLIENT_CLASS_NAME,
+                                           num_nodes=clients_num,
+                                           params={"thin_client_connection": thin_client_connection},
+                                           start_ignite = False,
+                                           startup_timeout_sec=180)
+
+        ignite.start()
+        static_clients.start()

Review comment:
       please, add await_started and await_finished checks here.
   You should use the following methods - https://github.com/apache/ignite/blob/8c432ba136b5bdb4e6c7e4b3265785b53959d036/modules/ducktests/tests/ignitetest/services/ignite_app.py#L53




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8968: IGNITE-13970 Thin client compatibility test

Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r607595744



##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -46,14 +46,17 @@ def __init__(self, context, config, java_class_name, num_nodes=1, params="", sta
         self.servicejava_class_name = servicejava_class_name
         self.java_class_name = java_class_name
         self.params = params
+        self.start_ignite = start_ignite

Review comment:
       As I said, de-facto, this usage can and *should* be encapsulated at `IgniteAwareService`.`await_started()`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r606939162



##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,70 @@
+# 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.
+
+"""
+This module contains client tests
+"""
+import time
+
+from ducktape.mark import parametrize
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size

Review comment:
       wong comments.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] nizhikov commented on a change in pull request #8968: Ignite-13970-2

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8968:
URL: https://github.com/apache/ignite/pull/8968#discussion_r606939375



##########
File path: modules/ducktests/tests/ignitetest/tests/thin_client_test.py
##########
@@ -0,0 +1,70 @@
+# 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.
+
+"""
+This module contains client tests
+"""
+import time
+
+from ducktape.mark import parametrize
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+class ThinClientTest(IgniteTest):
+    """
+    cluster - cluster size
+    CACHE_NAME - name of the cache to create for the test.
+    JAVA_CLIENT_CLASS_NAME - running classname.
+    static_clients - the number of permanently employed clients.
+    """
+
+    JAVA_CLIENT_CLASS_NAME = "org.apache.ignite.internal.ducktest.tests.thin_client_test.ThinClientSelfTest"
+
+    """
+    Tests services implementations
+    """
+
+    @cluster(num_nodes=2)
+    @ignite_versions(str(DEV_BRANCH))
+    @parametrize(num_nodes=2, clients_num=1)
+    def test_replicated_atomic_cache(self, ignite_version, num_nodes, clients_num):

Review comment:
       wrong test name.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org