Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature-11129] [API] Refactor org.apache.dolphinscheduler.api.controller.ProcessDefinitionController #11180

Conversation

MichaelDeSteven
Copy link
Contributor

@MichaelDeSteven MichaelDeSteven commented Jul 27, 2022

Purpose of the pull request

close #11129

change log

  • refactor ProcessDefinitionService interface:use Result to replace Map<String, Object>
  • add WorkflowDefinitionController

@MichaelDeSteven MichaelDeSteven changed the title refactor: process definition service [refactor-11129]: process definition service Jul 27, 2022
@MichaelDeSteven MichaelDeSteven changed the title [refactor-11129]: process definition service [refactor-11129][api] process definition service Jul 27, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2022

Codecov Report

Merging #11180 (093248c) into dev (181cd2f) will increase coverage by 0.00%.
The diff coverage is 38.11%.

❗ Current head 093248c differs from pull request most recent head 40011b9. Consider uploading reports for the commit 40011b9 to get more accurate results

@@            Coverage Diff            @@
##                dev   #11180   +/-   ##
=========================================
  Coverage     40.24%   40.24%           
- Complexity     4925     4927    +2     
=========================================
  Files           982      982           
  Lines         37558    37548   -10     
  Branches       4127     4124    -3     
=========================================
- Hits          15115    15113    -2     
+ Misses        20908    20902    -6     
+ Partials       1535     1533    -2     
Impacted Files Coverage Δ
...che/dolphinscheduler/api/python/PythonGateway.java 20.00% <0.00%> (+0.18%) ⬆️
.../apache/dolphinscheduler/common/utils/S3Utils.java 0.00% <0.00%> (ø)
...r/server/master/service/MasterFailoverService.java 54.47% <0.00%> (-2.31%) ⬇️
...api/service/impl/ProcessDefinitionServiceImpl.java 32.20% <40.55%> (+0.47%) ⬆️
...er/api/controller/ProcessDefinitionController.java 38.57% <45.71%> (-6.78%) ⬇️
...cheduler/api/service/impl/ExecutorServiceImpl.java 40.09% <50.00%> (+0.47%) ⬆️
...r/api/service/impl/ProcessInstanceServiceImpl.java 60.10% <50.00%> (ø)
...che/dolphinscheduler/alert/AlertSenderService.java 55.39% <56.25%> (+0.65%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@MichaelDeSteven
Copy link
Contributor Author

@caishunfeng @SbloodyS this pr is ready to review. plz take a look.

@SbloodyS
Copy link
Member

SbloodyS commented Aug 2, 2022

I haven't seen any v2 controller in this PR. Is there something missing? @MichaelDeSteven

@MichaelDeSteven
Copy link
Contributor Author

I haven't seen any v2 controller in this PR. Is there something missing? @MichaelDeSteven

@SbloodyS the refactor work is so huge. I want to split the refactor task as small as possible. Hoping this way can make refactor task going well.
this pr is first step. After that, I will continue v2 controller task.

@SbloodyS
Copy link
Member

SbloodyS commented Aug 2, 2022

I haven't seen any v2 controller in this PR. Is there something missing? @MichaelDeSteven

@SbloodyS the refactor work is so huge. I want to split the refactor task as small as possible. Hoping this way can make refactor task going well. this pr is first step. After that, I will continue v2 controller task.

So we split the refactor task to a single function. Each pr should contain at least one refactor function and its related modifications. It is not recommended to submit irrelevant things in advance.

@MichaelDeSteven
Copy link
Contributor Author

MichaelDeSteven commented Aug 2, 2022

I haven't seen any v2 controller in this PR. Is there something missing? @MichaelDeSteven

@SbloodyS the refactor work is so huge. I want to split the refactor task as small as possible. Hoping this way can make refactor task going well. this pr is first step. After that, I will continue v2 controller task.

So we split the refactor task to a single function. Each pr should contain at least one refactor function and its related modifications. It is not recommended to submit irrelevant things in advance.

how about this, I will add some interface in v2 controller base on this pr. after the pr is merged, I will continue pr #11148 to finish the rest of interface?

@SbloodyS
Copy link
Member

SbloodyS commented Aug 2, 2022

If you think the refactor work is too large. You can submit a PR for each refactor method. @MichaelDeSteven

@MichaelDeSteven
Copy link
Contributor Author

If you think the refactor work is too large. You can submit a PR for each refactor method. @MichaelDeSteven

the tough part is solved in this pr. I will complete v2 controller task in this pr. code review may take some time. but that would be all right.

@SbloodyS
Copy link
Member

SbloodyS commented Aug 2, 2022

If you think the refactor work is too large. You can submit a PR for each refactor method. @MichaelDeSteven

the tough part is solved in this pr. I will complete v2 controller task in this pr. code review may take some time. but that would be all right.

In the mean time, you can convert this pr to draft. And mark it as ready to review after you finish your work.

@MichaelDeSteven MichaelDeSteven marked this pull request as draft August 2, 2022 07:58
@MichaelDeSteven MichaelDeSteven changed the title [refactor-11129][api] process definition service [Feature-11129] [API] Refactor create method of org.apache.dolphinscheduler.api.controller.ProcessDefinitionController Aug 4, 2022
@sonarcloud
Copy link

sonarcloud bot commented Aug 4, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 198 Code Smells

52.2% 52.2% Coverage
3.3% 3.3% Duplication

@MichaelDeSteven MichaelDeSteven marked this pull request as ready for review August 4, 2022 03:38
@MichaelDeSteven MichaelDeSteven changed the title [Feature-11129] [API] Refactor create method of org.apache.dolphinscheduler.api.controller.ProcessDefinitionController [Feature-11129] [API] Refactor org.apache.dolphinscheduler.api.controller.ProcessDefinitionController Aug 8, 2022
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Oct 24, 2023
@github-actions
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API] Refactor org.apache.dolphinscheduler.api.controller.ProcessDefinitionController
3 participants