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

chore: add a github "action-validator" in CI #28358

Merged
merged 5 commits into from
May 9, 2024
Merged

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented May 6, 2024

SUMMARY

#28343 fixes an issue where I had broken a specific github action workflow in a previous PR without realizing I did. If/when altering the yaml, if the yaml itsefl is valid (we have a check for that) yet the schema is not - either through passing the right data structure or required types, the action just won't run and the PR is still mergeable.

NOTE: it's really easy to break a github action (yaml is brittle, and schema enforcement for actions is loose), and it's also really easy to miss the error message about it. The failure gets somewhat buried on the Actions tab (at the bottom of https://github.com/apache/superset/actions), which is super easy to miss.

Originally I tried the pre-commit hook, but bailed on that approach as it requires installing action-validator in PATH, which is not super convenient as it's a Rust package. There's a node wrapper we leverage in this action, but didn't want to extend the dev deps beyond python/pip.

Find out more about the hook here: https://github.com/mpalmer/action-validator

TESTING

For testing, I reverted the change from #28343, and tested the bash script fails. It did and provided a nice json blob with everything wrong with the schema provided.

SCREENSHOT

Screenshot 2024-05-06 at 11 02 18 AM

#28343 fixes an issue where I had broken a specific github action workflow in a previous PR without realizing I did. If/when altering the yaml, if the yaml itsefl is valid (we have a check for that) yet the schema is not - either through passing the right data structure or required types, the action just won't run and the PR is still mergeable. The failure gets somewhat burried on the Actions tab (at the bottom of https://github.com/apache/superset/actions)

Here I'm hoping that this pre-commit hook will both catch this early, and prevent the PR from being mergeable.

Find out more about the hook here: https://github.com/mpalmer/action-validator
@pull-request-size pull-request-size bot added size/M and removed size/XS labels May 6, 2024
@github-actions github-actions bot added the github_actions Pull requests that update GitHub Actions code label May 6, 2024
@mistercrunch mistercrunch changed the title chore: add action-validator as pre-commit hook chore: add a github "action-validator" in CI May 6, 2024
@john-bodley john-bodley added review:checkpoint Last PR reviewed during the daily review standup review:draft labels May 6, 2024
@@ -19,8 +19,8 @@ on:
default: 'false'
description: Whether to force a latest tag on the release
options:
- true
- false
- 'true'
Copy link
Member Author

@mistercrunch mistercrunch May 6, 2024

Choose a reason for hiding this comment

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

NOTE: it caught this as an error so I fixed it

@mistercrunch mistercrunch marked this pull request as ready for review May 6, 2024 18:06
@john-bodley john-bodley removed the review:checkpoint Last PR reviewed during the daily review standup label May 7, 2024
Copy link
Member

@eschutho eschutho left a comment

Choose a reason for hiding this comment

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

nice!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 8, 2024
@mistercrunch mistercrunch merged commit 499fb2c into master May 9, 2024
57 of 58 checks passed
@mistercrunch mistercrunch deleted the action_validator branch May 9, 2024 15:26
imancrsrk pushed a commit to imancrsrk/superset that referenced this pull request May 10, 2024
jingyi-zhao-01 pushed a commit to jingyi-zhao-01/superset that referenced this pull request May 16, 2024
EnxDev pushed a commit to EnxDev/superset that referenced this pull request May 31, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.1.0 labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels github_actions Pull requests that update GitHub Actions code lgtm This PR has been approved by a maintainer preset-io size/M 🚢 4.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants