-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add github action to publish artifacts for presto stable release #24821
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
Conversation
| needs: publish-release-tag | ||
| if: | | ||
| (github.event.inputs.publish_maven == 'true' || github.event.inputs.publish_image == 'true' || github.event.inputs.publish_docs == 'true') && | ||
| (success() || (github.event.inputs.publish_release_tag != 'true' && !failure() && !cancelled())) |
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'm thinking about the second part of the condition
(success() || (github.event.inputs.publish_release_tag != 'true' && !failure() && !cancelled()))
Would it be easier to check for
- if the previous job was selected to run
github.event.inputs.publish_release_tag == 'truethen success() is required to continue - is this a true statement? - if it was not selected to run the state of the job doesn't matter (aka skipped).
aka
github.event.inputs.publish_release_tag == 'true' && success() || (github.event.inputs.publish_release_tag != 'true')
Is this is what is intended? If so I think it is more readable and logical to understand. My brain is a pretzel with the other expression.
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.
Thanks @czentgr for your suggestion, the condition in the workflow did looks very complex. So I did the refactoring based on your comments, now the code feels very intuitive. (BTW, success() not working well when job is skipped).
I think we both agree that:
- All jobs should succeed and should be able to cancel, so we can use
(!failure() && !cancelled())to ensure there are no failures and the user has not canceled the workflow.. - Based on the choices, we will add a condition to check the flags.
The updated condition is as follows:
(!failure() &&!cancelled()) &&
(github.event.inputs.publish_maven == 'true' || github.event.inputs.publish_image == 'true' || github.event.inputs.publish_docs == 'true')
| needs: publish-release-tag | ||
| if: | | ||
| github.event.inputs.publish_native_image == 'true' && | ||
| (success() || (github.event.inputs.publish_release_tag != 'true' && !failure() && !cancelled())) |
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.
Here it is trying to skip the fact that the previous step could be skipped by it not being selected. So success() would fail.
Also this job can run even if the job in the needs: didn't run, right?
I'm wondering if we could leave out the success(), failed() cancelled() because those are working on previous steps and instead can check for selection flags and explicit job results instead.
if (github.event.inputs.publish_release_tag == 'true' && jobs.publish-release-tag.results == 'success' ||
github.event.inputs.publish_release_tag != 'true')
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.
As I said in your latest comment, the condition is updated as:
(!failure() && !cancelled()) && github.event.inputs.publish_image == 'true'
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.
Now all the conditions are updated as:
(!failure() && !cancelled()) && github.event.inputs.XXX == 'true'
a20bf7e to
a2e3c7f
Compare
|
@czentgr I updated the conditions, please help to review when you have time, thanks. |
|
@czentgr I know you've been busy, but would appreciate a review. This PR is currently blocking the 292 release. If you'd like me to take a pass let me know |
czentgr
left a comment
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.
Thanks. Yes, thanks for the changes. This is much easier to understand when a step runs.
success() is not true if the previous step was skipped.
But now we execute the next step if success or skip and the step is selected. So that's good to abort if there was a failure or cancellation.
|
Thank you for your careful review @czentgr |
…stodb#24821) ## Description This is a follow up of PR prestodb#24697, seperate the github release actions into 2 PRs. This is the second one - publish release artifacts like jars, docker images, websites, etc. The document of stable release process can be found in https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months) <!---Describe your changes in detail--> Prior to the coming release, there were 5 steps: - [Step 1 - Create a release branch and increment the version in the master branch](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-1-create-a-release-branch-and-increment-the-version-in-the-master-branch) - [Step 2 - Create release notes PR](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)) - [Step 3: clean up the tag](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-3-clean-up-the-tag) - [Step 4: Publish Maven and Docker artifacts](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-4-publish-maven-and-docker-artifacts) - [Step 5: Publish docs website](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-5-publish-docs-website) ### Action "Presto Stable Release -Publish" <img width="969" alt="presto_release_publish_workflow" src="https://github.com/user-attachments/assets/31e941ac-c3b9-4a2d-9660-1e9bfaaf2e26" /> - [Step 3: clean up the tag](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-3-clean-up-the-tag) - [Step 4: Publish Maven and Docker artifacts](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-4-publish-maven-and-docker-artifacts) - [Step 5: Publish docs website](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-5-publish-docs-website) #### Parameters <img width="309" alt="presto_release_publish_inputs" src="https://github.com/user-attachments/assets/54e18451-e99c-43b0-9921-a90983edd592" /> #### Jobs  This PR required prestodb/presto-release-tools#47, prestodb/prestodb.github.io#291 to be merged and released a new version`presto-release-tools` of before using these 2 actions ## Motivation and Context ## Impact Presto stable release ## Test Plan - Tested with my env - Publish => https://github.com/unix280/presto/actions/runs/13844025717 - Test with new release 0.292 after merged ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
…stodb#24821) ## Description This is a follow up of PR prestodb#24697, seperate the github release actions into 2 PRs. This is the second one - publish release artifacts like jars, docker images, websites, etc. The document of stable release process can be found in https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months) <!---Describe your changes in detail--> Prior to the coming release, there were 5 steps: - [Step 1 - Create a release branch and increment the version in the master branch](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-1-create-a-release-branch-and-increment-the-version-in-the-master-branch) - [Step 2 - Create release notes PR](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)) - [Step 3: clean up the tag](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-3-clean-up-the-tag) - [Step 4: Publish Maven and Docker artifacts](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-4-publish-maven-and-docker-artifacts) - [Step 5: Publish docs website](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-5-publish-docs-website) ### Action "Presto Stable Release -Publish" <img width="969" alt="presto_release_publish_workflow" src="https://github.com/user-attachments/assets/31e941ac-c3b9-4a2d-9660-1e9bfaaf2e26" /> - [Step 3: clean up the tag](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-3-clean-up-the-tag) - [Step 4: Publish Maven and Docker artifacts](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-4-publish-maven-and-docker-artifacts) - [Step 5: Publish docs website](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-5-publish-docs-website) #### Parameters <img width="309" alt="presto_release_publish_inputs" src="https://github.com/user-attachments/assets/54e18451-e99c-43b0-9921-a90983edd592" /> #### Jobs  This PR required prestodb/presto-release-tools#47, prestodb/prestodb.github.io#291 to be merged and released a new version`presto-release-tools` of before using these 2 actions ## Motivation and Context ## Impact Presto stable release ## Test Plan - Tested with my env - Publish => https://github.com/unix280/presto/actions/runs/13844025717 - Test with new release 0.292 after merged ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
…stodb#24821) ## Description This is a follow up of PR prestodb#24697, seperate the github release actions into 2 PRs. This is the second one - publish release artifacts like jars, docker images, websites, etc. The document of stable release process can be found in https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months) <!---Describe your changes in detail--> Prior to the coming release, there were 5 steps: - [Step 1 - Create a release branch and increment the version in the master branch](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-1-create-a-release-branch-and-increment-the-version-in-the-master-branch) - [Step 2 - Create release notes PR](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)) - [Step 3: clean up the tag](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-3-clean-up-the-tag) - [Step 4: Publish Maven and Docker artifacts](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-4-publish-maven-and-docker-artifacts) - [Step 5: Publish docs website](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-5-publish-docs-website) ### Action "Presto Stable Release -Publish" <img width="969" alt="presto_release_publish_workflow" src="https://github.com/user-attachments/assets/31e941ac-c3b9-4a2d-9660-1e9bfaaf2e26" /> - [Step 3: clean up the tag](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-3-clean-up-the-tag) - [Step 4: Publish Maven and Docker artifacts](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-4-publish-maven-and-docker-artifacts) - [Step 5: Publish docs website](https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)#step-5-publish-docs-website) #### Parameters <img width="309" alt="presto_release_publish_inputs" src="https://github.com/user-attachments/assets/54e18451-e99c-43b0-9921-a90983edd592" /> #### Jobs  This PR required prestodb/presto-release-tools#47, prestodb/prestodb.github.io#291 to be merged and released a new version`presto-release-tools` of before using these 2 actions ## Motivation and Context ## Impact Presto stable release ## Test Plan - Tested with my env - Publish => https://github.com/unix280/presto/actions/runs/13844025717 - Test with new release 0.292 after merged ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
Description
This is a follow up of PR #24697, seperate the github release actions into 2 PRs. This is the second one - publish release artifacts like jars, docker images, websites, etc.
The document of stable release process can be found in https://github.com/prestodb/presto/wiki/Stable-release-process-(every-2-months)
Prior to the coming release, there were 5 steps:
Action "Presto Stable Release -Publish"
Parameters
Jobs
This PR required prestodb/presto-release-tools#47, prestodb/prestodb.github.io#291 to be merged and released a new version
presto-release-toolsof before using these 2 actionsMotivation and Context
Impact
Presto stable release
Test Plan
Contributor checklist
Release Notes