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

[Improvement][Api] Adding transaction when update process definition #5546

Closed
ruanwenjun opened this issue May 25, 2021 · 7 comments
Closed
Assignees
Labels
good first issue good first issue improvement make more easy to user or prompt friendly Stale

Comments

@ruanwenjun
Copy link
Member

Describe the question
When we update a process definition, we need to perform multiple DML operations, it is needed to add transaction to ensure the data is consistent when an exception occurs.

public int saveProcessDefinition(User operator, Project project, String name, String desc, String locations,

There are also some other method need to check, such as:

public void saveTaskRelation(User operator, ProcessTaskRelation processTaskRelation) {

public int saveTaskDefinition(User operator, Long projectCode, TaskNode taskNode, TaskDefinition taskDefinition) {

Which version of DolphinScheduler:
-[dev]

Describe alternatives you've considered
Add transaction, and it is recommended to split ProcessService into CommandService、ProcessDefinitionService、ProcessInstanceService、TaskService

@github-actions github-actions bot added the Waiting for reply Waiting for reply label May 25, 2021
@CalvinKirs CalvinKirs added help wanted Extra attention is needed improvement make more easy to user or prompt friendly volunteer wanted good first issue good first issue and removed Waiting for reply Waiting for reply help wanted Extra attention is needed labels May 27, 2021
@apache apache deleted a comment from github-actions bot May 27, 2021
@ji04xiaogang
Copy link
Contributor

I would like to contribute to this.

@HomminLee
Copy link
Contributor

I want to take this issue.

@brave-lee
Copy link
Contributor

brave-lee commented Nov 11, 2021

@ruanwenjun Please confirm that the current interface already supports transactions

@HomminLee
Copy link
Contributor

@ruanwenjun I have a question. Class ProcessService exists in project [dolphinscheduler-service], and there are already exists ProcessDefinitionService、ProcessInstanceService、TaskInstanceService in project [dolphinscheduler-api]. In my previous thoughts, I want split ProcessService to those service class. But I found ProcessService also be used by project [dolphinscheduler-server]. If I follow the previous ideas to complete, the project [dolphinscheduler-api] cannot use those service, this is unacceptable.
So, do you have any suggestions? Should I just create 4 new class in project [dolphinscheduler-service] with other names?

@ruanwenjun
Copy link
Member Author

@HomminLee You can create a new issue to track your idea.

@github-actions
Copy link

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.

@github-actions github-actions bot added the Stale label Feb 20, 2022
@github-actions
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue good first issue improvement make more easy to user or prompt friendly Stale
Projects
None yet
Development

No branches or pull requests

5 participants