You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Pochatkin (via GitHub)" <gi...@apache.org> on 2026/01/13 14:45:53 UTC

[PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

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

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


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

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

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


Re: [PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #7402:
URL: https://github.com/apache/ignite-3/pull/7402#discussion_r2686763548


##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/cluster/unit/DeployUnitCall.java:
##########
@@ -96,6 +124,95 @@ public CompletableFuture<CallOutput<String>> execute(DeployUnitCallInput input)
         });
     }
 
+    private static File createZipFromDirectory(Path sourceDir) throws IOException {
+        Path zipPath = Files.createTempFile("deploy-unit-", ".zip");
+        try (OutputStream os = Files.newOutputStream(zipPath);
+                ZipOutputStream zos = new ZipOutputStream(os)) {
+            try (Stream<Path> stream = Files.walk(sourceDir)) {
+                stream.filter(Files::isRegularFile).forEach(filePath -> {
+                    Path relativePath = sourceDir.relativize(filePath);
+                    try {
+                        ZipEntry zipEntry = new ZipEntry(relativePath.toString().replace('\\', '/'));
+                        zos.putNextEntry(zipEntry);
+                        Files.copy(filePath, zos);
+                        zos.closeEntry();
+                    } catch (IOException e) {
+                        throw new RuntimeException("Failed to add file to ZIP: " + filePath, e);

Review Comment:
   Maybe use `sneakyThrow` here? This exception won't be caught by the `catch (IOException`



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

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

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


Re: [PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #7402:
URL: https://github.com/apache/ignite-3/pull/7402#discussion_r2687207560


##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/unit/ItDeploymentUnitTest.java:
##########
@@ -323,6 +323,80 @@ void deploySemantic() {
         });
     }
 
+    @Test
+    @DisplayName("Should deploy a unit from directory with subdirectories using --recursive")
+    void deployRecursive() throws IOException {
+        // Given a directory with subdirectories
+        Path recursiveDir = Files.createDirectory(WORK_DIR.resolve("recursive-test"));
+        Path subDir = Files.createDirectory(recursiveDir.resolve("subdir"));
+        Files.createFile(recursiveDir.resolve("root-file.txt"));
+        Files.createFile(subDir.resolve("nested-file.txt"));
+
+        // When deploy with --recursive option
+        execute("cluster", "unit", "deploy", "test.unit.recursive.1", "--version", "1.0.0",
+                "--path", recursiveDir.toString(), "--recursive");
+
+        // Then
+        assertAll(
+                this::assertExitCodeIsZero,
+                this::assertErrOutputIsEmpty,
+                () -> assertOutputContains("Done")
+        );
+
+        // And unit is deployed
+        await().untilAsserted(() -> {
+            execute("cluster", "unit", "list", "--plain", "test.unit.recursive.1");
+
+            assertDeployed("test.unit.recursive.1");
+        });
+    }
+
+    @Test
+    @DisplayName("Should deploy a unit from deeply nested directory using --recursive")
+    void deployRecursiveDeepNesting() throws IOException {
+        // Given a directory with deeply nested subdirectories
+        Path deepDir = Files.createDirectory(WORK_DIR.resolve("deep-test"));
+        Path level1 = Files.createDirectory(deepDir.resolve("level1"));
+        Path level2 = Files.createDirectory(level1.resolve("level2"));
+        Path level3 = Files.createDirectory(level2.resolve("level3"));
+        Files.createFile(deepDir.resolve("root.txt"));
+        Files.createFile(level1.resolve("file1.txt"));
+        Files.createFile(level2.resolve("file2.txt"));
+        Files.createFile(level3.resolve("file3.txt"));
+
+        // When deploy with --recursive option
+        execute("cluster", "unit", "deploy", "test.unit.recursive.2", "--version", "1.0.0",
+                "--path", deepDir.toString(), "--recursive");
+
+        // Then
+        assertAll(
+                this::assertExitCodeIsZero,
+                this::assertErrOutputIsEmpty,
+                () -> assertOutputContains("Done")
+        );
+
+        // And unit is deployed
+        await().untilAsserted(() -> {
+            execute("cluster", "unit", "list", "--plain", "test.unit.recursive.2");
+
+            assertDeployed("test.unit.recursive.2");
+        });
+    }
+
+    @Test
+    @DisplayName("Should display error when --recursive is used with a file path")

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.

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

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


Re: [PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #7402:
URL: https://github.com/apache/ignite-3/pull/7402#discussion_r2686902723


##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/cluster/unit/DeployUnitCall.java:
##########
@@ -64,27 +67,52 @@ public CompletableFuture<CallOutput<String>> execute(DeployUnitCallInput input)
         if (Files.notExists(path)) {
             return completedFuture(DefaultCallOutput.failure(new FileNotFoundException(path.toString())));
         }
+
+        DeploymentContent content;
+        try {
+            content = input.recursive() ? createZipContent(path) : createFilesContent(path);

Review Comment:
   Fixed



##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/cluster/unit/DeployUnitCall.java:
##########
@@ -96,6 +124,95 @@ public CompletableFuture<CallOutput<String>> execute(DeployUnitCallInput input)
         });
     }
 
+    private static File createZipFromDirectory(Path sourceDir) throws IOException {
+        Path zipPath = Files.createTempFile("deploy-unit-", ".zip");
+        try (OutputStream os = Files.newOutputStream(zipPath);
+                ZipOutputStream zos = new ZipOutputStream(os)) {
+            try (Stream<Path> stream = Files.walk(sourceDir)) {
+                stream.filter(Files::isRegularFile).forEach(filePath -> {
+                    Path relativePath = sourceDir.relativize(filePath);
+                    try {
+                        ZipEntry zipEntry = new ZipEntry(relativePath.toString().replace('\\', '/'));
+                        zos.putNextEntry(zipEntry);
+                        Files.copy(filePath, zos);
+                        zos.closeEntry();
+                    } catch (IOException e) {
+                        throw new RuntimeException("Failed to add file to ZIP: " + filePath, e);

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.

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

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


Re: [PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #7402:
URL: https://github.com/apache/ignite-3/pull/7402#discussion_r2686984735


##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/unit/ItDeploymentUnitTest.java:
##########
@@ -323,6 +323,80 @@ void deploySemantic() {
         });
     }
 
+    @Test
+    @DisplayName("Should deploy a unit from directory with subdirectories using --recursive")
+    void deployRecursive() throws IOException {
+        // Given a directory with subdirectories
+        Path recursiveDir = Files.createDirectory(WORK_DIR.resolve("recursive-test"));
+        Path subDir = Files.createDirectory(recursiveDir.resolve("subdir"));
+        Files.createFile(recursiveDir.resolve("root-file.txt"));
+        Files.createFile(subDir.resolve("nested-file.txt"));
+
+        // When deploy with --recursive option
+        execute("cluster", "unit", "deploy", "test.unit.recursive.1", "--version", "1.0.0",
+                "--path", recursiveDir.toString(), "--recursive");
+
+        // Then
+        assertAll(
+                this::assertExitCodeIsZero,
+                this::assertErrOutputIsEmpty,
+                () -> assertOutputContains("Done")
+        );
+
+        // And unit is deployed
+        await().untilAsserted(() -> {
+            execute("cluster", "unit", "list", "--plain", "test.unit.recursive.1");
+
+            assertDeployed("test.unit.recursive.1");
+        });
+    }
+
+    @Test
+    @DisplayName("Should deploy a unit from deeply nested directory using --recursive")
+    void deployRecursiveDeepNesting() throws IOException {
+        // Given a directory with deeply nested subdirectories
+        Path deepDir = Files.createDirectory(WORK_DIR.resolve("deep-test"));
+        Path level1 = Files.createDirectory(deepDir.resolve("level1"));
+        Path level2 = Files.createDirectory(level1.resolve("level2"));
+        Path level3 = Files.createDirectory(level2.resolve("level3"));
+        Files.createFile(deepDir.resolve("root.txt"));
+        Files.createFile(level1.resolve("file1.txt"));
+        Files.createFile(level2.resolve("file2.txt"));
+        Files.createFile(level3.resolve("file3.txt"));
+
+        // When deploy with --recursive option
+        execute("cluster", "unit", "deploy", "test.unit.recursive.2", "--version", "1.0.0",
+                "--path", deepDir.toString(), "--recursive");
+
+        // Then
+        assertAll(
+                this::assertExitCodeIsZero,
+                this::assertErrOutputIsEmpty,
+                () -> assertOutputContains("Done")
+        );
+
+        // And unit is deployed
+        await().untilAsserted(() -> {
+            execute("cluster", "unit", "list", "--plain", "test.unit.recursive.2");
+
+            assertDeployed("test.unit.recursive.2");
+        });
+    }
+
+    @Test
+    @DisplayName("Should display error when --recursive is used with a file path")

Review Comment:
   There's the same test in the `DeployCommandTest`, I don't think there's much value in duplicating these.



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

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

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


Re: [PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin merged PR #7402:
URL: https://github.com/apache/ignite-3/pull/7402


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

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

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


Re: [PR] IGNITE-27548 Add recursive option to deploy unit command [ignite-3]

Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #7402:
URL: https://github.com/apache/ignite-3/pull/7402#discussion_r2686756212


##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/call/cluster/unit/DeployUnitCall.java:
##########
@@ -64,27 +67,52 @@ public CompletableFuture<CallOutput<String>> execute(DeployUnitCallInput input)
         if (Files.notExists(path)) {
             return completedFuture(DefaultCallOutput.failure(new FileNotFoundException(path.toString())));
         }
+
+        DeploymentContent content;
+        try {
+            content = input.recursive() ? createZipContent(path) : createFilesContent(path);

Review Comment:
   Let's move the return here and merge declaration and assignment



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