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

fix(3181): Virtual builds not completed when triggered by webhook #3261

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

sagar1312
Copy link
Member

Context

Changes made in the PR #3252 introduced a regression. Virtual job builds at the beginning of the event workflow never got marked as completed (remain in CREATED state) when the event is created by SCM webhook.

Objective

Mark virtual job builds at the beginning of the event workflow for all the scenarios.

References

#3181

License

I confirm that this contribution is made under a BSD license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@sagar1312 sagar1312 force-pushed the fix-3181-virtual_job_by_webhook branch from 75611e8 to 645634d Compare January 2, 2025 18:02
@coveralls
Copy link

coveralls commented Jan 2, 2025

Coverage Status

coverage: 95.336% (+0.007%) from 95.329%
when pulling 5448a07 on fix-3181-virtual_job_by_webhook
into 696ce8c on master.

@sagar1312 sagar1312 force-pushed the fix-3181-virtual_job_by_webhook branch from 2bab9a0 to 798b4fd Compare January 2, 2025 18:20
@jithine
Copy link
Member

jithine commented Jan 2, 2025

Is this a duplicate of #3260

@sagar1312
Copy link
Member Author

Is this a duplicate of #3260

Yes, #3260 can be closed. Moved the logic to mark virtual builds as "SUCCESS" to a common module and use it wherever needed.

@sagar1312 sagar1312 force-pushed the fix-3181-virtual_job_by_webhook branch from 798b4fd to 5448a07 Compare January 2, 2025 18:52
Comment on lines +30 to +42
for (const build of virtualJobBuilds) {
await updateBuildAndTriggerDownstreamJobs(
{
status: Status.SUCCESS,
statusMessage: BUILD_STATUS_MESSAGES.SKIP_VIRTUAL_JOB.statusMessage,
statusMessageType: BUILD_STATUS_MESSAGES.SKIP_VIRTUAL_JOB.statusMessageType
},
build,
server,
username,
scmContext
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (const build of virtualJobBuilds) {
await updateBuildAndTriggerDownstreamJobs(
{
status: Status.SUCCESS,
statusMessage: BUILD_STATUS_MESSAGES.SKIP_VIRTUAL_JOB.statusMessage,
statusMessageType: BUILD_STATUS_MESSAGES.SKIP_VIRTUAL_JOB.statusMessageType
},
build,
server,
username,
scmContext
);
}
await Promise.all(
virtualJobBuilds.map(build =>
updateBuildAndTriggerDownstreamJobs(
{
status: Status.SUCCESS,
statusMessage: BUILD_STATUS_MESSAGES.SKIP_VIRTUAL_JOB.statusMessage,
statusMessageType: BUILD_STATUS_MESSAGES.SKIP_VIRTUAL_JOB.statusMessageType
},
build,
server,
username,
scmContext
)
)
);

Copy link
Member Author

Choose a reason for hiding this comment

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

Besides marking the status of virtual build to "SUCCESS", we also process its downstream jobs. While triggering the downstream jobs in the workflow, we have been following depth-first approach. Sticking to the same here as well.
So we need process all the virtual builds sequentially.

@sagar1312 sagar1312 merged commit 39f4292 into master Jan 3, 2025
2 checks passed
@sagar1312 sagar1312 deleted the fix-3181-virtual_job_by_webhook branch January 3, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants