-
Notifications
You must be signed in to change notification settings - Fork 3
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: let one job require be join #45
fix: let one job require be join #45
Conversation
// Calculate which upstream jobs trigger the current job | ||
const upstreamOr = new Set(requires.filter(name => name.charAt(0) === '~')); | ||
const upstreamAnd = new Set(requires.filter(name => name.charAt(0) !== '~')); | ||
const isJoin = upstreamAnd.size >= 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The essence of this PR is here.
- const isJoin = upstreamAnd.size > 1;
+ const isJoin = upstreamAnd.size >= 1;
For a single job in requires, do we treat it as a join, or just a regular external trigger ? Is this PR addressing the scenario where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small suggestions
Co-authored-by: Tiffany K <tiffanykyi@gmail.com>
Co-authored-by: Tiffany K <tiffanykyi@gmail.com>
Yes. We treat it as join case.
I think YES. Currently, On the other hands, |
🎉 This PR is included in version 4.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@y-oksaku this is causing some undesirable behavior when 2 pipelines has multiple single job triggers against each other. We would work an a sample workflow and share further details |
So what's happening is that prior to this change, something like this
would have triggered job Y2, on completion of build for job Ref: |
@jithine jobs:
hub:
requires: [ ~commit, ~pr ]
test:
requires: []
test2:
requires: []
Y2:
requires: [ ~sd@10:foo, test, test2 ] However, with this PR, since a job is now treated as a join even when there is only one, the situation described in the comment has occurred. |
Context
When the following settings are made, the
join
job does not run upon restarting the build frommain
. This is due to the fact that when a Remote Join is composed of only one job, it is not treated as a Remote Join and it is also not triggered as~sd@456:sub
.Objective
This PR will make a correction to treat it as a join condition even if it consists of only one job.
Once it is treated as a Remote Join, it will no longer be excluded at the part below during restart, allowing the build to run correctly.
https://github.com/screwdriver-cd/screwdriver/blob/c1f15b7647dc4360093e9df64d9db69131718640/plugins/builds/index.js#L1009
References
License
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.