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

[DSIP-67] Use command to trigger workflow instance rather generate workflow instance #16523

Conversation

ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented Aug 27, 2024

Purpose of the pull request

close #16485

Brief change log

  • Add interface in master to manualTrigger/scheduleTrigger/backfillTrigger a workflow.
  • Move all workflow instance construct logic to master.
  • API will directly call master's api to control a workflow.
  • Return workflow instance id when triggering a workflow.
  • Delete TriggerRelation, since we can directly use workflow instance id to track workflow instance.

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@github-actions github-actions bot added UI ui and front end related backend test labels Aug 27, 2024
@SbloodyS SbloodyS added this to the 3.3.0 milestone Aug 27, 2024
@SbloodyS SbloodyS added the DSIP label Aug 27, 2024
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_moveGenerateWorkflowInstance2Master branch from 75d8821 to cf8ba93 Compare August 27, 2024 07:18
}

private void doBackfillDependentWorkflow(final BackfillWorkflowCommandParam backfillWorkflowCommandParam,
final Command backfillCommand) {
private void doBackfillDependentWorkflow(final BackfillWorkflowDTO backfillWorkflowDTO,

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'backfillWorkflowDTO' is never used.
private void doBackfillDependentWorkflow(final BackfillWorkflowCommandParam backfillWorkflowCommandParam,
final Command backfillCommand) {
private void doBackfillDependentWorkflow(final BackfillWorkflowDTO backfillWorkflowDTO,
final List<String> backfillTimeList) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'backfillTimeList' is never used.
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

CI failed. Please check it. @ruanwenjun

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_moveGenerateWorkflowInstance2Master branch 3 times, most recently from 43b6bd0 to 244b7f2 Compare August 27, 2024 12:07
@github-actions github-actions bot added the e2e e2e test label Aug 27, 2024
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_moveGenerateWorkflowInstance2Master branch from 244b7f2 to 6a3505f Compare August 27, 2024 12:08
SbloodyS
SbloodyS previously approved these changes Aug 27, 2024
Copy link
Member

@SbloodyS SbloodyS 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.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_moveGenerateWorkflowInstance2Master branch from 6a3505f to b9d4032 Compare August 27, 2024 14:20
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_moveGenerateWorkflowInstance2Master branch from b9d4032 to 5674832 Compare August 27, 2024 14:28
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_moveGenerateWorkflowInstance2Master branch from 5674832 to 8fec4a4 Compare August 27, 2024 15:12
Copy link

sonarcloud bot commented Aug 27, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
5.6% Coverage on New Code (required ≥ 60%)

See analysis details on SonarCloud

Copy link
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 20a0e50 into apache:dev Aug 28, 2024
66 of 69 checks passed
@ruanwenjun ruanwenjun deleted the dev_wenjun_moveGenerateWorkflowInstance2Master branch August 28, 2024 15:31
@LiJie20190102
Copy link

Will this PR merge into section 3.1.9?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend DSIP e2e e2e test test UI ui and front end related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DSIP-67] Use command to trigger workflow instance rather generate workflow instance
4 participants