[Fix-17026][api] Fix the schedule-release-state from import workflowDefinition whose scheduled-release-state was online #17132
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes the schedule-release-state when importing a workflow definition by ensuring that online schedules trigger an update via the scheduler API.
- Updated test cases to cover various import scenarios (e.g., unauthorized project access, empty file content, missing parameters, duplicate names)
- Revised Mockito argument usage in tests and added a new condition in the service implementation to trigger schedule updates when the release state is ONLINE.
Reviewed Changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkflowDefinitionServiceTest.java | Added tests to validate workflow definition import and updated Mockito matchers. |
| dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkflowDefinitionServiceImpl.java | Included a conditional call to the scheduler API when the schedule’s release state is ONLINE. |
Files not reviewed (3)
- dolphinscheduler-api/src/test/resources/workflowImport/check_duplicate_name.json: Language not supported
- dolphinscheduler-api/src/test/resources/workflowImport/check_importance_params.json: Language not supported
- dolphinscheduler-api/src/test/resources/workflowImport/check_successful.json: Language not supported
| if (ReleaseState.ONLINE.equals(schedule.getReleaseState())) { | ||
| Project project = projectMapper.queryByCode(projectCode); | ||
| schedulerApi.insertOrUpdateScheduleTask(project.getId(), schedule); | ||
| } |
There was a problem hiding this comment.
I think we don't need this since we don't allow to copy the online schedule workflow definition. We should keep the same logic.
There was a problem hiding this comment.
I see what you mean. We should throw an exception when the releaseState is online. This would prevent user confusion.
What do you think?
There was a problem hiding this comment.
I think we should just import workflow with offline state. And marked in the document.
There was a problem hiding this comment.
LGTM, thanks for the suggestion!
| "environmentCode" : -1, | ||
| "environmentName" : null | ||
| } | ||
| } ] |
There was a problem hiding this comment.
Keep the blank in the last line.
| "environmentCode" : -1, | ||
| "environmentName" : null | ||
| } | ||
| } ] |
There was a problem hiding this comment.
Keep the blank in the last line.
yingh0ng
force-pushed
the
Fix-17026-import
branch
from
7a3278b to
bde4688
Compare
yingh0ng
force-pushed
the
Fix-17026-import
branch
from
b6d6fb1 to
a84467d
Compare
Purpose of the pull request
close #17026
Brief change log
Verify this pull request
This change added tests and can be verified as follows:
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md