tristaZero commented on a change in pull request #10121:
URL: https://github.com/apache/shardingsphere/pull/10121#discussion_r615515334
##########
File path:
shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/process/GovernanceExecuteProcessReporter.java
##########
@@ -29,12 +35,14 @@
public final class GovernanceExecuteProcessReporter implements
ExecuteProcessReporter {
@Override
- public void report(final SQLStatementContext<?> context, final
ExecutionGroupContext<? extends SQLExecutionUnit> executionGroupContext, final
ExecuteProcessConstants constants) {
- // TODO :Call API of configCenter
+ public void report(final SQLStatementContext<?> context, final
ExecutionGroupContext<? extends SQLExecutionUnit> executionGroupContext) {
+ ExecuteProcessContext executeProcessContext = new
ExecuteProcessContext(executionGroupContext);
+ ShardingSphereEventBus.getInstance().post(new
ReportExecuteProcessSummaryEvent(new
YamlExecuteProcessContext(executeProcessContext)));
Review comment:
Why is a new class, i.e.,`YamlExecuteProcessContext` used here?
`ExecutionGroupContext` is unsuitable?
##########
File path:
shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/process/GovernanceExecuteProcessReporter.java
##########
@@ -29,12 +35,14 @@
public final class GovernanceExecuteProcessReporter implements
ExecuteProcessReporter {
@Override
- public void report(final SQLStatementContext<?> context, final
ExecutionGroupContext<? extends SQLExecutionUnit> executionGroupContext, final
ExecuteProcessConstants constants) {
- // TODO :Call API of configCenter
+ public void report(final SQLStatementContext<?> context, final
ExecutionGroupContext<? extends SQLExecutionUnit> executionGroupContext) {
+ ExecuteProcessContext executeProcessContext = new
ExecuteProcessContext(executionGroupContext);
+ ShardingSphereEventBus.getInstance().post(new
ReportExecuteProcessSummaryEvent(new
YamlExecuteProcessContext(executeProcessContext)));
Review comment:
It is suggested to consider renaming `ReportExecuteProcessUnitEvent`
with `ExecuteProcessUnitReportEvent `. Similarly,
`ReportExecuteProcessSummaryEvent` -> `ExecuteProcessSummaryReportEvent `.
##########
File path:
shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/process/spi/ExecuteProcessReporter.java
##########
@@ -31,9 +31,8 @@
* Report the summary of this task.
* @param context context
* @param executionGroupContext execution group context
- * @param constants constants
*/
- void report(SQLStatementContext<?> context, ExecutionGroupContext<?
extends SQLExecutionUnit> executionGroupContext, ExecuteProcessConstants
constants);
+ void report(SQLStatementContext<?> context, ExecutionGroupContext<?
extends SQLExecutionUnit> executionGroupContext);
Review comment:
Is there no possibility that `ExecuteProcessConstants` is another value?
How about report stoping, resuming status?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
