Skip to content

[Feature][API] New restful API for workflow and schedule #11912

Merged
caishunfeng merged 13 commits intoapache:devfrom
zhongjiajie:f-openapi-wf-schedule
Sep 23, 2022
Merged

[Feature][API] New restful API for workflow and schedule #11912
caishunfeng merged 13 commits intoapache:devfrom
zhongjiajie:f-openapi-wf-schedule

Conversation

@zhongjiajie
Copy link
Copy Markdown
Member

@zhongjiajie zhongjiajie commented Sep 13, 2022 •
edited
Loading

CURD for workflow and schedule, different with exists
API, this new restful api only operate single resource
in each request, and return the latest. For example,
previous workflow should also need to post tasks definition
and tasks relation definition, but this patch will allow
you to create workflow without task relate information

@SbloodyS SbloodyS added this to the 4.0.0-alpha milestone Sep 14, 2022
@jieguangzhou jieguangzhou mentioned this pull request Sep 14, 2022
Closed
18 tasks
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Sep 15, 2022 •
edited
Loading

This PR is not equal to refactored exists API as #10257 do, we also separate existingprocess definition and schedule into single resources CRUD(previous process definition also will submit tasks and task relation). And we make it more Restful

@SbloodyS SbloodyS mentioned this pull request Sep 15, 2022
Closed
3 tasks
@zhongjiajie zhongjiajie marked this pull request as ready for review September 16, 2022 02:09
@caishunfeng caishunfeng changed the title [feat] New restful API for workflow and schedule [Feature][Api] New restful API for workflow and schedule Sep 16, 2022
@caishunfeng caishunfeng changed the title [Feature][Api] New restful API for workflow and schedule [Feature][API] New restful API for workflow and schedule Sep 16, 2022
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 16, 2022 •
edited
Loading

Codecov Report

Merging #11912 (9e88f87) into dev (6eb1eb7) will increase coverage by 0.37%.
The diff coverage is 67.62%.

@@             Coverage Diff              @@
##                dev   #11912      +/-   ##
============================================
+ Coverage     38.69%   39.07%   +0.37%     
- Complexity     4010     4082      +72     
============================================
  Files          1001     1010       +9     
  Lines         37422    37696     +274     
  Branches       4262     4326      +64     
============================================
+ Hits          14480    14728     +248     
+ Misses        21295    21281      -14     
- Partials       1647     1687      +40
Impacted Files Coverage Δ
...eduler/api/dto/schedule/ScheduleFilterRequest.java 0.00% <0.00%> (ø)
...scheduler/api/controller/ScheduleV2Controller.java 9.09% <9.09%> (ø)
...eduler/api/dto/workflow/WorkflowFilterRequest.java 33.33% <33.33%> (ø)
...cheduler/api/service/impl/ExecutorServiceImpl.java 43.73% <37.50%> (+0.39%) ⬆️
...eduler/api/dto/schedule/ScheduleUpdateRequest.java 41.66% <41.66%> (ø)
...er/api/controller/ProcessDefinitionController.java 51.04% <50.00%> (+1.53%) ⬆️
...eduler/api/dto/workflow/WorkflowUpdateRequest.java 50.00% <50.00%> (ø)
...api/service/impl/ProcessDefinitionServiceImpl.java 33.81% <61.90%> (+2.43%) ⬆️
...heduler/api/service/impl/SchedulerServiceImpl.java 26.77% <71.57%> (+17.89%) ⬆️
...scheduler/api/controller/WorkflowV2Controller.java 85.71% <85.71%> (ø)
... and 14 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zhongjiajie
[feat] New restful API for workflow and schedule
6d53617 
CURD for workflow and schedule, different with exists
API, this new restful api only operate single resource
in each request, and return the latest. For example,
previous workflow should also need to post tasks definition
and tasks relation definition, but this patch will allow
you to create workflow without task relate information
@zhongjiajie
Add license
978e064
@zhongjiajie
fix code scaning
4062bcc
@zhongjiajie
use checkProjectAndAuthThrowException, and fix CI error
2644963
@zhongjiajie
smell code
2224bb8
@zhongjiajie
recover code scanning fix
7a846a2 
fix the warning code
@zhongjiajie
fix conflict
c5ce4ce
@zhongjiajie zhongjiajie mentioned this pull request Sep 17, 2022
Merged
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Sep 17, 2022

fix conflict from adding log in #11782

@zhongjiajie zhongjiajie mentioned this pull request Sep 17, 2022
Closed
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Sep 19, 2022

PTAL if you have time @caishunfeng @SbloodyS

songjianet
songjianet previously approved these changes Sep 19, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@songjianet songjianet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

caishunfeng
caishunfeng requested changes Sep 19, 2022
View reviewed changes
...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java Outdated
* @param workflowCreateRequest the new workflow object will be created
* @return New ProcessDefinition object created just now
*/
ProcessDefinition createProcessDefinitionV2(User loginUser, WorkflowCreateRequest workflowCreateRequest);
Copy link
Copy Markdown
Contributor

@caishunfeng caishunfeng Sep 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the different between v1 and v2? Maybe v2 it's not a good guide for developers if v1 can not be replaced with v2.

Copy link
Copy Markdown
Member Author

@zhongjiajie zhongjiajie Sep 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1 is for currently API(operate workflow and task and task relation object), and V2 is only operate workflow object. V2 is mainly for RESTful api, which we only change one single object instand of too many object

Copy link
Copy Markdown
Contributor

@caishunfeng caishunfeng Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we use v2 uniformly in the future? including UI or support restful api or pyds?

Copy link
Copy Markdown
Member Author

@zhongjiajie zhongjiajie Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we can do that, and in that moment, maybe it it the best time to migrate python api into separate repository

ruanwenjun
ruanwenjun reviewed Sep 19, 2022
View reviewed changes
Tianqi-Dotes
Tianqi-Dotes previously approved these changes Sep 19, 2022
View reviewed changes
@zhongjiajie @caishunfeng
Update dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler…
99b6c39 
…/api/controller/ScheduleV2Controller.java

Co-authored-by: caishunfeng <caishunfeng2021@gmail.com>
@zhongjiajie zhongjiajie dismissed stale reviews from Tianqi-Dotes and songjianet via 99b6c39 September 21, 2022 08:50
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Sep 21, 2022

we still have discuss in #11912 (comment) and #11912 (comment)

@zhongjiajie
fix license issue and format code
4e58004
@zhongjiajie
Merge remote-tracking branch 'upstream/dev' into f-openapi-wf-schedule
61b5e28
@zhongjiajie
change method name
9e88f87 
from createProcessDefinitionV2 to createSingleProcessDefinition
from updateProcessDefinitionV2 to updateSingleProcessDefinition
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Sep 23, 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 10 Code Smells

74.3% 74.3% Coverage
2.5% 2.5% Duplication

caishunfeng
caishunfeng approved these changes Sep 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall

@caishunfeng caishunfeng merged commit 82ddd72 into apache:dev Sep 23, 2022
@caishunfeng
Copy link
Copy Markdown
Contributor

caishunfeng commented Sep 23, 2022

Thanks @zhongjiajie

@zhongjiajie zhongjiajie deleted the f-openapi-wf-schedule branch September 25, 2022 10:22
xdu-chenrj pushed a commit to xdu-chenrj/dolphinscheduler that referenced this pull request Oct 13, 2022
@zhongjiajie @caishunfeng @xdu-chenrj
[Feature][API] New restful API for workflow and schedule (apache#11912)
ff30cdc 
* [feat] New restful API for workflow and schedule

CURD for workflow and schedule, different with exists
API, this new restful api only operate single resource
in each request, and return the latest. For example,
previous workflow should also need to post tasks definition
and tasks relation definition, but this patch will allow
you to create workflow without task relate information

* use checkProjectAndAuthThrowException, and fix CI error

* Update dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ScheduleV2Controller.java

* change method name
from createProcessDefinitionV2 to createSingleProcessDefinition
from updateProcessDefinitionV2 to updateSingleProcessDefinition

Co-authored-by: caishunfeng <caishunfeng2021@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ruanwenjun ruanwenjun ruanwenjun left review comments

@caishunfeng caishunfeng caishunfeng approved these changes

@songjianet songjianet songjianet left review comments

@Tianqi-Dotes Tianqi-Dotes Tianqi-Dotes left review comments

@SbloodyS SbloodyS Awaiting requested review from SbloodyS SbloodyS is a code owner

Assignees

@zhongjiajie zhongjiajie

Labels

backend refactor

Projects

None yet

Milestone

4.0.0-alpha

Development

Successfully merging this pull request may close these issues.

8 participants

@zhongjiajie @codecov-commenter @caishunfeng @songjianet @ruanwenjun @Tianqi-Dotes @github-advanced-security @SbloodyS