You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/10/17 12:18:18 UTC
[GitHub] [dolphinscheduler] DarkAssassinator opened a new issue, #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
DarkAssassinator opened a new issue, #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403
### Search before asking
- [X] I had searched in the [issues](https://github.com/apache/dolphinscheduler/issues?q=is%3Aissue) and found no similar feature requirement.
### Description
**Why need refactor `ProcessServiceImpl`?**
The number of lines of code in `ProcessServiceImpl` exceed 3K+ lines,
+ This will bring great challenges to post-maintenance work.
+ Also it's a very bad experience for the reader.
+ Corresponding unit tests are more difficult to cover.
### Are you willing to submit a PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
--
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@dolphinscheduler.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1427178000
This issue has been closed because it has not received response for too long time. You could reopen it if you encountered similar problems in the future.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] DarkAssassinator commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1285635273
> +1, In fact, most of our logic is validate -> transform -> insert/update, we can use different sub class to to sub work. Please start with a small service e.g. WorkerGroupService.
big +1 I just found that there are too many services included in this class, we will migrate these other services to other small services.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] fuchanghai commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1285653926
> > > because the scope of this part of the refactoring is relatively large, so I split it into multiple sub-tasks, in order to control the scope of influence and facilitate review and merge. cc @EricGao888 @songjianet
> >
> >
> > @DarkAssassinator I'm +1 to the refactoring of `ProcessServiceImpl `. But `ProcessServiceImpl` is in one of the core modules, I think we should add related unit tests during the refactoring instead of after the refactoring.
> > BTW, may I ask whether you want to do all of these by yourself or need some more hands?
>
> Sure, one of the purposes of refactoring it is that the current test rate coverage is not enough. need some more hands? -> Sure, @fuchanghai will help me to refactor it, and welcome more hands to join in. And I plan to focus on refactoring this part for a while. If it is delayed for too long, it may cost more work later.
>
i'm happy to do it
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] github-actions[bot] closed issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
URL: https://github.com/apache/dolphinscheduler/issues/12403
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] EricGao888 commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
EricGao888 commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1285586865
> because the scope of this part of the refactoring is relatively large, so I split it into multiple sub-tasks, in order to control the scope of influence and facilitate review and merge. cc @EricGao888 @songjianet
@DarkAssassinator I'm +1 to the refactoring of `ProcessServiceImpl `. But `ProcessServiceImpl` is in one of the core modules, I think we should add related unit tests during the refactoring instead of after the refactoring.
BTW, may I ask whether you want to do all of these by yourself or need some more hands?
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] DarkAssassinator commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1285627292
> > because the scope of this part of the refactoring is relatively large, so I split it into multiple sub-tasks, in order to control the scope of influence and facilitate review and merge. cc @EricGao888 @songjianet
>
> @DarkAssassinator I'm +1 to the refactoring of `ProcessServiceImpl `. But `ProcessServiceImpl` is in one of the core modules, I think we should add related unit tests during the refactoring instead of after the refactoring.
>
> BTW, may I ask whether you want to do all of these by yourself or need some more hands?
Sure, one of the purposes of refactoring it is that the current test rate coverage is not enough.
need some more hands? -> Sure, @fuchanghai will help me to refactor it, and welcome more hands to join in. And I plan to focus on refactoring this part for a while. If it is delayed for too long, it may cost more work later.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1280838583
Thank you for your feedback, we have received your issue, Please wait patiently for a reply.
* In order for us to understand your request as soon as possible, please provide detailed information、version or pictures.
* If you haven't received a reply for a long time, you can [join our slack](https://s.apache.org/dolphinscheduler-slack) and send your question to channel `#troubleshooting`
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] DarkAssassinator commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1285532825
because the scope of this part of the refactoring is relatively large, so I split it into multiple sub-tasks, in order to control the scope of influence and facilitate review and merge. cc @EricGao888 @songjianet
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1418313768
This issue has been automatically marked as stale because it has not had recent activity for 30 days. It will be closed in next 7 days if no further activity occurs.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] fuchanghai commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1281691057
@DarkAssassinator can you relation #11228
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on issue #12403: [Improvement][Service] Refactor ProcessServiceImpl in service module.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on issue #12403:
URL: https://github.com/apache/dolphinscheduler/issues/12403#issuecomment-1285629384
+1, In fact, most of our logic is validate -> transform -> insert/update, we can use different sub class to to sub work.
Please start with a small service e.g. WorkerGroupService.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org