You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/11/03 07:15:36 UTC
[GitHub] [inlong] woofyzhao opened a new pull request, #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
woofyzhao opened a new pull request, #6388:
URL: https://github.com/apache/inlong/pull/6388
- Fixes #6387
### Motivation
- The auto push source type can be in status that does not make sense, for example, TO_BE_ISSUED_FROZEN.
- The auto push source fields are not correctly updated.
- Other adaptation issue.
### Modification
- Replace all temporary status with end status for auto push source type.
- Correctly update stream source fields and inlong stream fields.
- Remove stream operation on sink update (**open for discussion**)
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] woofyzhao commented on a diff in pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #6388:
URL: https://github.com/apache/inlong/pull/6388#discussion_r1013546957
##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sink/StreamSinkServiceImpl.java:
##########
@@ -254,15 +238,6 @@ public Boolean update(SinkRequest request, String operator) {
StreamSinkOperator sinkOperator = operatorFactory.getInstance(request.getSinkType());
sinkOperator.updateOpt(request, nextStatus, operator);
- // If the stream is [CONFIG_SUCCESSFUL], then asynchronously start the [CREATE_STREAM_RESOURCE] process
Review Comment:
So what's the appropriate time to restart after config updates ? Perhaps it's better be launched by end user in a separate call. According to the Single Responsibility Principle the service update handler should, in general, perform no other than data updating.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] woofyzhao commented on a diff in pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #6388:
URL: https://github.com/apache/inlong/pull/6388#discussion_r1013546957
##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sink/StreamSinkServiceImpl.java:
##########
@@ -254,15 +238,6 @@ public Boolean update(SinkRequest request, String operator) {
StreamSinkOperator sinkOperator = operatorFactory.getInstance(request.getSinkType());
sinkOperator.updateOpt(request, nextStatus, operator);
- // If the stream is [CONFIG_SUCCESSFUL], then asynchronously start the [CREATE_STREAM_RESOURCE] process
Review Comment:
So what's the appropriate time to restart after config updates ? Perhaps it's better be launched by end user in a separate call. According to the Single Responsibility Principle the service update handler should only perform no other than config updating.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] woofyzhao closed pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
woofyzhao closed pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
URL: https://github.com/apache/inlong/pull/6388
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow merged pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
healchow merged PR #6388:
URL: https://github.com/apache/inlong/pull/6388
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] woofyzhao commented on pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
woofyzhao commented on PR #6388:
URL: https://github.com/apache/inlong/pull/6388#issuecomment-1302897286
Needs further discussion. Close first.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] woofyzhao commented on a diff in pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #6388:
URL: https://github.com/apache/inlong/pull/6388#discussion_r1013546957
##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sink/StreamSinkServiceImpl.java:
##########
@@ -254,15 +238,6 @@ public Boolean update(SinkRequest request, String operator) {
StreamSinkOperator sinkOperator = operatorFactory.getInstance(request.getSinkType());
sinkOperator.updateOpt(request, nextStatus, operator);
- // If the stream is [CONFIG_SUCCESSFUL], then asynchronously start the [CREATE_STREAM_RESOURCE] process
Review Comment:
So what's the appropriate time to restart after config updates ? Perhaps it's better launched by end user in a separate call from front end. According to the Single Responsibility Principle the service update handler should, in general, perform no other than data updating.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] healchow commented on a diff in pull request #6388: [INLONG-6387][Manager] Adapt auto-push source status and fix source field update
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #6388:
URL: https://github.com/apache/inlong/pull/6388#discussion_r1013570037
##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sink/StreamSinkServiceImpl.java:
##########
@@ -125,15 +118,6 @@ public Integer save(SinkRequest request, String operator) {
sinkEntity.setStatus(nextStatus.getCode());
sinkMapper.updateStatus(sinkEntity);
}
- // If the stream is [CONFIG_SUCCESSFUL], then asynchronously start the [CREATE_STREAM_RESOURCE] process
Review Comment:
Suggest adding a param to control whether to enable the process.
I will commit it in a new PR.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org