denis-chudov commented on code in PR #6604:
URL: https://github.com/apache/ignite-3/pull/6604#discussion_r2387889925
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/exception/EmptyDataNodesException.java:
##########
@@ -15,17 +15,25 @@
* limitations under the License.
*/
-package org.apache.ignite.internal.cluster.management;
+package org.apache.ignite.internal.distributionzones.exception;
-import static
org.apache.ignite.lang.ErrorGroups.CommonConfiguration.CONFIGURATION_VALIDATION_ERR;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
+import static
org.apache.ignite.lang.ErrorGroups.DistributionZones.EMPTY_DATA_NODES_ERR;
-import org.apache.ignite.internal.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteException;
/**
- * Exception representing invalid node configuration.
+ * Exception thrown when there are no data nodes for the given zone.
*/
-public class InvalidNodeConfigurationException extends IgniteInternalException
{
- public InvalidNodeConfigurationException(String message) {
- super(CONFIGURATION_VALIDATION_ERR, message);
+public class EmptyDataNodesException extends IgniteException {
+ private static final long serialVersionUID = 5691362165660196984L;
+
+ /**
+ * Constructor.
+ *
+ * @param zoneId Zone ID.
+ */
+ public EmptyDataNodesException(int zoneId) {
+ super(EMPTY_DATA_NODES_ERR, format("Empty data nodes for zone
[zoneId={}].", zoneId));
Review Comment:
1. I am not sure it is user-facing, it's internal exception showing the
failure of some internal mechanics, like ReplicationTimeoutException or
PrimaryReplicaAwaitTimeoutException. It should be wrapped into public
user-facing exception like SqlException and may be root cause of it.
2.
3. because of 1, I think it's not necessary, and it would complicate the
code because we would need additional dependency for resolving the zone name by
id.
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/exception/EmptyDataNodesException.java:
##########
@@ -15,17 +15,25 @@
* limitations under the License.
*/
-package org.apache.ignite.internal.cluster.management;
+package org.apache.ignite.internal.distributionzones.exception;
-import static
org.apache.ignite.lang.ErrorGroups.CommonConfiguration.CONFIGURATION_VALIDATION_ERR;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
+import static
org.apache.ignite.lang.ErrorGroups.DistributionZones.EMPTY_DATA_NODES_ERR;
-import org.apache.ignite.internal.lang.IgniteInternalException;
+import org.apache.ignite.lang.IgniteException;
/**
- * Exception representing invalid node configuration.
+ * Exception thrown when there are no data nodes for the given zone.
*/
-public class InvalidNodeConfigurationException extends IgniteInternalException
{
- public InvalidNodeConfigurationException(String message) {
- super(CONFIGURATION_VALIDATION_ERR, message);
+public class EmptyDataNodesException extends IgniteException {
+ private static final long serialVersionUID = 5691362165660196984L;
+
+ /**
+ * Constructor.
+ *
+ * @param zoneId Zone ID.
+ */
+ public EmptyDataNodesException(int zoneId) {
+ super(EMPTY_DATA_NODES_ERR, format("Empty data nodes for zone
[zoneId={}].", zoneId));
Review Comment:
1. I am not sure it is user-facing, it's internal exception showing the
failure of some internal mechanics, like ReplicationTimeoutException or
PrimaryReplicaAwaitTimeoutException. It should be wrapped into public
user-facing exception like SqlException and may be root cause of it.
2. because of 1, I think it's not necessary, and it would complicate the
code because we would need additional dependency for resolving the zone name by
id.
--
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]
