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 2020/07/31 14:59:16 UTC

[GitHub] [ignite] anton-vinogradov opened a new pull request #8109: Json params support

anton-vinogradov opened a new pull request #8109:
URL: https://github.com/apache/ignite/pull/8109


   


----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       Oh, I see. I thought that JsonNode is a generic, but it's not. So, there is a choose between plain JsonNode and DTO. In favor of latter:
   1.  Currently there are not, but in one day could be complex json with nested objects and arrays. User will have to use `mapper.readValue(jsonNode, DTO.class)` if need it. 
   2. If you want change code of your app in one day and change params, there won't help from IDE for you. There is a possibility of NPE in jsonNode.get(). DTO provides checks for such situations (code fails in compile time).
   3. All tests have own package, so I don't see problems with additional classes that are isolated within a package.




----------------------------------------------------------------
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 merged pull request #8109: Json params support

Posted by GitBox <gi...@apache.org>.
anton-vinogradov merged pull request #8109:
URL: https://github.com/apache/ignite/pull/8109


   


----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       Oh, I see. I thought that JsonNode is a generic, but it's not. So, there is a choice between plain JsonNode and DTO. In favor of latter:
   1.  Currently there are not, but in one day could be complex json with nested objects and arrays. User will have to use `mapper.readValue(jsonNode, DTO.class)` if need it. 
   2. If you want change code of your app in one day and change params, there won't help from IDE for you. There is a possibility of NPE in jsonNode.get(). DTO provides checks for such situations (code fails in compile time).
   3. All tests have own package, so I don't see problems with additional classes that are isolated within a package.




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       "reader" makes next line more readable ))))




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(Map<String, String> args) {

Review comment:
       Done




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(Map<String, String> args) {

Review comment:
       Why do you use Map instead of fair JsonNode object? Now you're limited with flat structure, also user is responsible for casting value types. 




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       Max, I suppose that this is more robust solution to problem. Constantly escape everything is a little bit tiresome. base64 solves this problem radically.




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       mapper.readTree(new String(Base64.getDecoder().decode(params[2]), UTF_8))




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Ok, I'd think it should be part of infrastructure spec.




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       ok, understand.




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       It will be unique for every app, will this lead to useless classes?




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       For example, JSON contains "," already used as a separator at ignite_aware.py because of necessary to transfer some params where only one allowed.




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(Map<String, String> args) {

Review comment:
       +1 JsonObject seems to be more universal solution and has a potential to extend




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       Just use single quotes then. This is enough, check how run_tests.sh do the same:
   https://github.com/apache/ignite/blob/ignite-ducktape/modules/ducktests/tests/docker/run_tests.sh#L136




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       Yep




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       It will be unique for every app, this will lead to useless classes?




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       For every old version?




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(Map<String, String> args) {

Review comment:
       Why do you use Map instead of fair JsonNode object? Now you're limited with flat structure and user responsible to cast value types. 

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \

Review comment:
       Looks like user_libs and service_libs should be lists and shouldn't know about colons delimeter. As colon is a part of USER_LIBS, now we have usage of the delimiter in 3 places (user_libs, service_libs, USER_LIBS)
   
   

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Why do you extract jackson jars from the AWS library? As I can see AWS isn't used in the application

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       Why do you use base64 there? Is it a workaround to avoid escape issue?

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_path.py
##########
@@ -56,3 +56,9 @@ def script(self, script_name):
         :return: absolute path to the specified script
         """
         return os.path.join(self._home, "bin", script_name)
+
+    def home(self):

Review comment:
       just rename _home to home and use it




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Why do not keep them in pom.xml of ducktests module?




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_path.py
##########
@@ -56,3 +56,9 @@ def script(self, script_name):
         :return: absolute path to the specified script
         """
         return os.path.join(self._home, "bin", script_name)
+
+    def home(self):

Review comment:
       Done




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Oh, now I understand. But do we need use strict link to AWS project, as we can't guarantee that it wouldn't be changed in future? Could we just use the `find` util to locate jackson jars anywhere in optional libs? 




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       Unused varoiable, I suppose

##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       Unused varoiable, I presume

##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       Unused variable, I presume




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \

Review comment:
       Done




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -82,7 +84,7 @@ def app_args(self):
         args = self.java_class_name + "," + IgniteAwareApplicationService.CONFIG_FILE
 
         if self.params != "":
-            args += "," + self.params
+            args += "," + str(base64.b64encode(json.dumps(self.params).encode("UTF-8")))

Review comment:
       Yes

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Because we should gain it somewhere, but it always presented as a dependency and always have the same version for all cases (set inside parent.pom) 




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       How to share java DTO to python? Seems we should add and sync DTO couple for each application. 
   How to sync this automatically?
   Seem overcomplicated...?




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Comment added. Can we postpone ideal resolving (locate jackson jars anywhere in optional libs) and just use AWS as source for now?




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Oh, now I understand. But do we need use strict link to AWS project, as we can't guarantee that it won't be changed in future? Could we just use the `find` util to locate jackson jars anywhere in optional libs? 




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py
##########
@@ -106,7 +108,23 @@ def env(self):
         return "export MAIN_CLASS={main_class}; ".format(main_class=self.servicejava_class_name) + \
                "export EXCLUDE_TEST_CLASSES=true; " + \
                "export IGNITE_LOG_DIR={log_dir}; ".format(log_dir=self.PERSISTENT_ROOT) + \
-               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*; " % self.user_libs
+               "export USER_LIBS=%s:/opt/ignite-dev/modules/ducktests/target/*%s; " % \
+               (self.user_libs, self.app_service_libs())
+
+    def app_service_libs(self):
+        """
+        :return: Libs required to start IgniteAwareApplication java implementation.
+        """
+        if self.version.is_dev:
+            return ""
+
+        libs = ""
+
+        for line in self.nodes[0].account.ssh_capture(
+                "ls -d %s/libs/optional/ignite-aws/* | grep jackson | tr '\n' ':' | sed 's/.$//'" % self.path.home()):

Review comment:
       Also there must be a comment with description of solution




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       Additional classes are ok. In case of complex hierarchy I'd prefer use generics (for example smth like `obj.getList().get(0).getRange()`) instead of ugly constructions (smth like `node.get("list").asArray()[0].get("range").asInt()`). 
   
   DTOs make code much much clearer, also it provides sanity checks out of the box.




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       Is it possible to use a DTO object instead? Generics is more clear solution




----------------------------------------------------------------
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] ivandasch commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       I think, that we should make in the bright future a generic application, that accepts broad range of generic options to load data to cache. For example, cache name, key-value schema (binary object), range for keys, number of keys, etc.
   
   For this argument DTO is a preferrable solution, I suppose. But we can implement this a little bit later as a separate task.




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       Not sure I've got an idea.
   Could you provide an example related to the current tests?




----------------------------------------------------------------
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] timoninmaxim commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       There still python, shame :( There are different approaches. Python just use dict and user is responsible for everything that is running in python code. Java could provide more strict checks. 
   
   Ok, agree, not an issue, as python won't directly use DTO object.




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       Which 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] anton-vinogradov commented on a change in pull request #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/DataGenerationApplication.java
##########
@@ -34,13 +35,13 @@ public DataGenerationApplication(Ignite ignite) {
     }
 
     /** {@inheritDoc} */
-    @Override protected void run(String[] args) {
+    @Override protected void run(JsonNode jsonNode) {
         log.info("Creating cache...");
 
-        IgniteCache<Integer, Integer> cache = ignite.createCache(args[0]);
+        IgniteCache<Integer, Integer> cache = ignite.createCache(jsonNode.get("cacheName").asText());
 
         try (IgniteDataStreamer<Integer, Integer> stmr = ignite.dataStreamer(cache.getName())) {
-            for (int i = 0; i < Integer.parseInt(args[1]); i++) {
+            for (int i = 0; i < jsonNode.get("range").asInt(); i++) {

Review comment:
       Great, let's re-discuss this later




----------------------------------------------------------------
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 #8109: Json params support

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



##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/utils/IgniteAwareApplicationService.java
##########
@@ -40,8 +47,15 @@ public static void main(String[] args) throws Exception {
 
         IgniteAwareApplication app = (IgniteAwareApplication)clazz.getConstructor().newInstance();
 
-        String[] appParams = Arrays.copyOfRange(params, 1, params.length);
+        ObjectMapper mapper = new ObjectMapper();
+
+        ObjectReader reader = mapper.readerFor(Map.class);

Review comment:
       Fixed




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