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 2022/12/07 09:25:30 UTC

[GitHub] [ignite-3] Flaugh24 opened a new pull request, #1419: IGNITE-18239: Exclude 'compute' and 'raft' from completions on 'node …

Flaugh24 opened a new pull request, #1419:
URL: https://github.com/apache/ignite-3/pull/1419

   https://issues.apache.org/jira/browse/IGNITE-18239


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

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


[GitHub] [ignite-3] isapego merged pull request #1419: IGNITE-18239: Exclude 'compute' and 'raft' from completions on 'node …

Posted by GitBox <gi...@apache.org>.
isapego merged PR #1419:
URL: https://github.com/apache/ignite-3/pull/1419


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

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


[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #1419: IGNITE-18239: Exclude 'compute' and 'raft' from completions on 'node …

Posted by GitBox <gi...@apache.org>.
PakhomovAlexander commented on code in PR #1419:
URL: https://github.com/apache/ignite-3/pull/1419#discussion_r1042168129


##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/core/repl/completer/DynamicCompleterActivationPoint.java:
##########
@@ -55,10 +57,22 @@ public void activateDynamicCompleter(DynamicCompleterRegistry registry) {
         );
         registry.register(
                 CompleterConf.builder()
-                        .command("node", "config", "show")
-                        .command("node", "config", "update").build(),
+                        .command("node", "config", "show").build(),
                 nodeConfigDynamicCompleterFactory
         );
+
+        registry.register(
+                CompleterConf.builder()
+                        .command("node", "config", "update")
+                        .filter((unused, candidates) -> {

Review Comment:
   This is a good idea to define a filter as a lambda right here but this approach has an issue: it is not covered with unit tests. You can define a separate class like `NodeConfigUpdateFilter` and test it with unite tests.



##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/core/repl/completer/DynamicCompleterActivationPoint.java:
##########
@@ -55,10 +57,22 @@ public void activateDynamicCompleter(DynamicCompleterRegistry registry) {
         );
         registry.register(
                 CompleterConf.builder()
-                        .command("node", "config", "show")
-                        .command("node", "config", "update").build(),
+                        .command("node", "config", "show").build(),
                 nodeConfigDynamicCompleterFactory
         );
+
+        registry.register(
+                CompleterConf.builder()
+                        .command("node", "config", "update")
+                        .filter((unused, candidates) -> {
+                            Set<String> exclusions = Set.of("compute", "raft");

Review Comment:
   Could you check please the case: `compute.`, `raft.`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

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