You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "yunbow30944 (via GitHub)" <gi...@apache.org> on 2026/01/22 05:30:06 UTC
[PR] [AINode] More accurate exceptions [iotdb]
yunbow30944 opened a new pull request, #17061:
URL: https://github.com/apache/iotdb/pull/17061
This pr adds more accurate exceptions for AINode.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AINode] More accurate exceptions [iotdb]
Posted by "CRZbulabula (via GitHub)" <gi...@apache.org>.
CRZbulabula merged PR #17061:
URL: https://github.com/apache/iotdb/pull/17061
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AINode] More accurate exceptions [iotdb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17061:
URL: https://github.com/apache/iotdb/pull/17061#discussion_r2716162083
##########
iotdb-core/ainode/iotdb/ainode/core/rpc/handler.py:
##########
@@ -92,6 +92,11 @@ def loadModel(self, req: TLoadModelReq) -> TSStatus:
status = self._ensure_model_is_registered(req.existingModelId)
if status.code != TSStatusCode.SUCCESS_STATUS.value:
return status
+ if set(req.deviceIdList) != req.deviceIdList:
Review Comment:
The comparison `set(req.deviceIdList) != req.deviceIdList` is incorrect for detecting duplicates in a Python list. In Python, comparing a set to a list will always evaluate to True (not equal) even when there are no duplicates, because they are different types. The correct approach is to compare the length of the list with the length of the set: `len(set(req.deviceIdList)) != len(req.deviceIdList)`.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/udf/UDTFForecast.java:
##########
@@ -214,8 +215,7 @@ private TsBlock forecast() throws Exception {
new TForecastReq(model_id, serde.serialize(inputData), outputLength)
.setOptions(options));
} catch (Exception e) {
- throw new IoTDBRuntimeException(
- e.getMessage(), TSStatusCode.CAN_NOT_CONNECT_AINODE.getStatusCode());
+ throw new AINodeConnectionException();
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
```suggestion
throw new AINodeConnectionException(e);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/function/tvf/ForecastTableFunction.java:
##########
@@ -564,7 +564,7 @@ protected TsBlock forecast() {
new TForecastReq(modelId, SERDE.serialize(inputData), outputLength)
.setOptions(options));
} catch (Exception e) {
- throw new IoTDBRuntimeException(e.getMessage(), CAN_NOT_CONNECT_AINODE.getStatusCode());
+ throw new AINodeConnectionException();
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
```suggestion
throw new AINodeConnectionException(e);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3706,7 +3707,7 @@ public SettableFuture<ConfigTaskResult> createModel(String modelId, String uri)
future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS));
}
} catch (final TException | ClientManagerException e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/function/tvf/ClassifyTableFunction.java:
##########
@@ -368,7 +368,7 @@ private TsBlock classify() {
CLIENT_MANAGER.borrowClient(AINodeClientManager.AINODE_ID_PLACEHOLDER)) {
resp = client.forecast(new TForecastReq(modelId, SERDE.serialize(inputData), outputLength));
} catch (Exception e) {
- throw new IoTDBRuntimeException(e.getMessage(), CAN_NOT_CONNECT_AINODE.getStatusCode());
+ throw new AINodeConnectionException();
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
```suggestion
throw new AINodeConnectionException(e);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3744,7 +3745,7 @@ public SettableFuture<ConfigTaskResult> showModels(final String modelId) {
}
ShowModelsTask.buildTsBlock(resp, future);
} catch (final Exception e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3799,7 +3800,7 @@ public SettableFuture<ConfigTaskResult> loadModel(
future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS));
}
} catch (final Exception e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3780,7 +3781,7 @@ public SettableFuture<ConfigTaskResult> showAIDevices() {
}
ShowAIDevicesTask.buildTsBlock(resp, future);
} catch (final Exception e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/ainode/iotdb/ainode/core/rpc/handler.py:
##########
@@ -104,6 +109,11 @@ def unloadModel(self, req: TUnloadModelReq) -> TSStatus:
status = self._ensure_model_is_registered(req.modelId)
if status.code != TSStatusCode.SUCCESS_STATUS.value:
return status
+ if set(req.deviceIdList) != req.deviceIdList:
Review Comment:
The comparison `set(req.deviceIdList) != req.deviceIdList` is incorrect for detecting duplicates in a Python list. In Python, comparing a set to a list will always evaluate to True (not equal) even when there are no duplicates, because they are different types. The correct approach is to compare the length of the list with the length of the set: `len(set(req.deviceIdList)) != len(req.deviceIdList)`.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/process/ai/InferenceOperator.java:
##########
@@ -248,7 +249,7 @@ private void submitInferenceTask() {
modelInferenceDescriptor.getModelId(), serde.serialize(inputTsBlock))
.setInferenceAttributes(modelInferenceDescriptor.getInferenceAttributes()));
} catch (Exception e) {
- throw new ModelInferenceProcessException(e.getMessage());
+ throw new AINodeConnectionException();
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
```suggestion
throw new AINodeConnectionException(e);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3763,7 +3764,7 @@ public SettableFuture<ConfigTaskResult> showLoadedModels(List<String> deviceIdLi
}
ShowLoadedModelsTask.buildTsBlock(resp, future);
} catch (final Exception e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3851,7 +3852,7 @@ public SettableFuture<ConfigTaskResult> createTuningTask(
future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS));
}
} catch (final Exception e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3818,7 +3819,7 @@ public SettableFuture<ConfigTaskResult> unloadModel(
future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS));
}
} catch (final Exception e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -3723,7 +3724,7 @@ public SettableFuture<ConfigTaskResult> dropModel(final String modelId) {
future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS));
}
} catch (final ClientManagerException | TException e) {
- future.setException(e);
+ future.setException(new AINodeConnectionException());
Review Comment:
The new AINodeConnectionException discards the original exception cause, which loses important debugging information such as the root cause of the connection failure (e.g., network timeout, authentication failure, etc.). Consider adding a constructor that accepts a Throwable cause parameter and pass the original exception when creating the AINodeConnectionException, similar to how IoTDBRuntimeException supports cause chaining.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org