zhoujinsong commented on code in PR #4116:
URL: https://github.com/apache/amoro/pull/4116#discussion_r2911168139
##########
amoro-common/src/main/java/org/apache/amoro/process/ProcessFactory.java:
##########
@@ -49,6 +50,8 @@ default ProcessTriggerStrategy triggerStrategy(TableFormat
format, Action action
return ProcessTriggerStrategy.METADATA_TRIGGER;
}
+ void availableExecuteEngines(Collection<ExecuteEngine> allAvailableEngines);
Review Comment:
这个新增方法缺少 JavaDoc 注释，需要说明：
1. 该方法的用途 —— `ProcessFactory` 实现类收到可用引擎列表后应该做什么？
2. 参数含义 —— `allAvailableEngines` 是当前全部已注册引擎，还是与本 factory 兼容的子集？
3. 是否需要 `default` 空实现 —— 目前现有的 `ProcessFactory`
实现类都未实现这个方法，会导致编译失败；如果该方法是可选的，建议加 `default void
availableExecuteEngines(Collection<ExecuteEngine> allAvailableEngines) {}`。
##########
amoro-common/src/main/java/org/apache/amoro/process/ProcessFactory.java:
##########
@@ -49,6 +50,8 @@ default ProcessTriggerStrategy triggerStrategy(TableFormat
format, Action action
return ProcessTriggerStrategy.METADATA_TRIGGER;
}
+ void availableExecuteEngines(Collection<ExecuteEngine> allAvailableEngines);
Review Comment:
This new method is missing a JavaDoc comment. Please clarify:
1. **Purpose** — What should a `ProcessFactory` implementation do when it
receives the available engine list? When is this called?
2. **Parameter semantics** — Does `allAvailableEngines` contain all
registered engines, or only those compatible with this factory?
3. **Default implementation** — Existing `ProcessFactory` implementations do
not override this method, which will cause compilation failures. If this method
is optional, please add a `default` empty implementation:
```java
default void availableExecuteEngines(Collection<ExecuteEngine>
allAvailableEngines) {}
```
--
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]
