-
Notifications
You must be signed in to change notification settings - Fork 2k
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 cancellation handling #7430
Conversation
/azp run java - core - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
799963d
to
03af12b
Compare
@@ -253,7 +253,7 @@ jobs: | |||
|
|||
- task: Maven@3 | |||
displayName: 'Start Jetty' | |||
condition: ne('${{ parameters.SDKType }}', 'client') | |||
condition: and(succeeded(), ne(variables['SdkType'], 'client')) |
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.
@mitchdenny why was this changed from parameters back to variables? Variables is going to try and pull from the pipeline variable SDKType which isn't set for every pipeline. Using the parameters should, in theory, guarantee that this only runs for the correct track.
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.
My bad ... I'll fix it up now. I was getting shown some interesting conflict resolutions when I did my rebase.
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.
I had a question about the Start Jetty task the change from '${{ parameters.SDKType }}' back to variables['SdkType'] in the conditions.
Fixed up in this PR: #7512 |
This PR extends/adjusts conditions on tasks and jobs to better handle cancellations. We have some tasks in jobs with an
always()
condition which effectively stops that job being cancelled. We need to avoid that in most cases because we generally need to cancel jobs in an build storm andalways()
conditions slow down recovery time.