ivanzlenko commented on code in PR #4681:
URL: https://github.com/apache/ignite-3/pull/4681#discussion_r1830689233
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/partitions/states/ItPartitionStatesTest.java:
##########
@@ -262,15 +265,16 @@ void testOutputFormatLocal() {
assertErrOutputIsEmpty();
assertOutputMatches(String.format(
- "Node name\tZone name\tTable name\tPartition
ID\tState\\r?\\n(%1$s)\t%2$s\t%2$s_table\t1\t(HEALTHY|AVAILABLE)\\r?\\n",
+ "Node name\t" + GLOBAL_PARTITION_STATE_FIELDS
Review Comment:
Combining concatenation with formatting makes it hard to read. I would've
add GLOBAL_PARTITION_STATE_FIELDS as another parameter to format function.
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/partitions/states/ItPartitionStatesTest.java:
##########
@@ -306,10 +310,10 @@ private void checkOutput(boolean global, Set<String>
zoneNames, Set<String> node
for (int i = 0; i < partitions; i++) {
assertOutputContains("\t" + i + "\t");
}
+ }
- for (int i = partitions; i < DEFAULT_PARTITION_COUNT; i++) {
- assertOutputDoesNotContain("\t" + i + "\t");
- }
+ if (partitions != 0) {
Review Comment:
Feels like it should be a part of an if above.
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/recovery/GlobalPartitionStateResponse.java:
##########
@@ -38,11 +40,15 @@ public class GlobalPartitionStateResponse {
@JsonCreator
public GlobalPartitionStateResponse(
@JsonProperty("partitionId") int partitionId,
Review Comment:
Can we have a more natural order here? Making partitionId as the first
parameter before nodeName could confuse a lot.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/LocalPartitionState.java:
##########
@@ -37,7 +43,16 @@ public class LocalPartitionState {
@IgniteToStringInclude
public final LocalPartitionStateEnum state;
- LocalPartitionState(String tableName, String zoneName, int partitionId,
LocalPartitionStateEnum state) {
+ LocalPartitionState(
+ int partitionId,
Review Comment:
The same about order
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/recovery/DisasterRecoveryController.java:
##########
@@ -133,14 +136,17 @@ private static GlobalPartitionStatesResponse
convertGlobalStates(Map<TablePartit
for (GlobalPartitionState state : globalStates.values()) {
states.add(new GlobalPartitionStateResponse(
state.partitionId,
- state.tableName,
state.zoneName,
+ state.tableId,
+ state.schemaName,
+ state.tableName,
state.state.name()
));
Review Comment:
The same about helper function
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/GlobalPartitionState.java:
##########
@@ -36,7 +42,16 @@ public class GlobalPartitionState {
@IgniteToStringInclude
public final GlobalPartitionStateEnum state;
- GlobalPartitionState(String tableName, String zoneName, int partitionId,
GlobalPartitionStateEnum state) {
+ GlobalPartitionState(
+ int partitionId,
Review Comment:
The same about order
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/recovery/LocalPartitionStateResponse.java:
##########
@@ -39,12 +41,16 @@ public class LocalPartitionStateResponse {
@JsonCreator
public LocalPartitionStateResponse(
@JsonProperty("partitionId") int partitionId,
- @JsonProperty("tableName") String tableName,
- @JsonProperty("zoneName") String zoneName,
@JsonProperty("nodeName") String nodeName,
+ @JsonProperty("zoneName") String zoneName,
+ @JsonProperty("tableId") int tableId,
+ @JsonProperty("schemaName") String schemaName,
+ @JsonProperty("tableName") String tableName,
@JsonProperty("state") String state
Review Comment:
Can we have a more natural order here? Making partitionId as the first
parameter before nodeName could confuse a lot.
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/recovery/DisasterRecoveryController.java:
##########
@@ -111,16 +111,19 @@ private static LocalPartitionStatesResponse
convertLocalStates(Map<TablePartitio
states.add(new LocalPartitionStateResponse(
state.partitionId,
- state.tableName,
- state.zoneName,
nodeName,
+ state.zoneName,
+ state.tableId,
+ state.schemaName,
+ state.tableName,
state.state.name()
));
Review Comment:
Can we wrap this constructor into a helper function?
Something like fromState. Too many same-type parameters could cause some
bugs.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
