IGNITE-27994 Create CLI command for REST endpoint to edit cluster name#7740
IGNITE-27994 Create CLI command for REST endpoint to edit cluster name#7740adityamukho wants to merge 15 commits intoapache:mainfrom
Conversation
| /** | ||
| * Tests for {@link ClusterRenameCall}. | ||
| */ | ||
| public class ItRenameCallTest extends CliIntegrationTest { |
There was a problem hiding this comment.
| public class ItRenameCallTest extends CliIntegrationTest { | |
| public class ItClusterRenameCallTest extends CliIntegrationTest { |
| } | ||
| } | ||
|
|
||
| private DefaultCallOutput<String> updateClusterConfig(ClusterManagementApi api, ClusterRenameCallInput input) |
There was a problem hiding this comment.
I would inline these methods
|
|
||
| try { | ||
| return updateClusterConfig(client, input); | ||
| } catch (ApiException | IllegalArgumentException e) { |
There was a problem hiding this comment.
| } catch (ApiException | IllegalArgumentException e) { | |
| } catch (ApiException) { |
don't see where would illegal argument come from
| import picocli.CommandLine.Parameters; | ||
|
|
||
| /** | ||
| * Mixin class for cluster name option in REPL mode. |
There was a problem hiding this comment.
| * Mixin class for cluster name option in REPL mode. | |
| * Mixin class for cluster name. |
| ) | ||
| private String name; | ||
|
|
||
| private String nameFromArgs() { |
There was a problem hiding this comment.
I wonder if we really need to provide both options (--name / just as an arg)
There was a problem hiding this comment.
I'll keep the --name option, since it is immune to argument order.
| /** | ||
| * Tests for {@link ClusterRenameCall}. | ||
| */ | ||
| public class ItRenameCallTest extends CliIntegrationTest { |
There was a problem hiding this comment.
Discussed with Vadim Pakhnushev, there is no point in separate tests for calls. Please replace with a test for command
ALso we need tests for different cases. Quoted, unquoted, empty name, etc
There was a problem hiding this comment.
I think it makes sense to have separate tests for calls. Testing only at the command level could hide potential issues with the call itself. There are existing call tests for other calls as well in the codebase, and for the same reason, I think they should also be retained.
There was a problem hiding this comment.
ALso we need tests for different cases. Quoted, unquoted, empty name, etc
👍
|
|
||
| /** Name that will be updated. */ | ||
| @Mixin | ||
| private ClusterNameMixin nameFromArgs; |
There was a problem hiding this comment.
| private ClusterNameMixin nameFromArgs; | |
| private ClusterNameMixin name; |
| import org.junit.jupiter.params.provider.Arguments; | ||
| import org.junit.jupiter.params.provider.MethodSource; | ||
|
|
||
| class RenameTest extends CliCommandTestBase { |
There was a problem hiding this comment.
| class RenameTest extends CliCommandTestBase { | |
| class ClusterRenameCommandTest extends CliCommandTestBase { |
|
|
||
| private static Stream<Arguments> calls() { | ||
| return Stream.of( | ||
| arguments( |
| Class<IT> callInputClass, | ||
| Function<IT, String> nameFunction | ||
| ) { | ||
| checkParameters(command, callClass, callInputClass, nameFunction, "test1 test2", "test1 test2"); |
There was a problem hiding this comment.
let's use parameterizedtest to unite these tests, passing args / expected name
2 participants
https://issues.apache.org/jira/browse/IGNITE-27994