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

CI: Fix using check_source flags as bool #121848

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

webknjaz
Copy link
Contributor

Previously, those flags would sometimes end up having empty string values, which tends to break evaluating them as JSON. This patch adds false fallbacks to all such outputs.

This allows feeding them to fromJSON() without a fear of them causing surprising internal behaviors in the GitHub Actions CI/CD workflows platform itself [1]. The behavior observed was that some skipped jobs wouldn't show up in the workflow sidebar view at all, would display in the graph view as Waiting for pending jobs and in the ${{ needs }} context, they would have a result: failure entry [2].

This should help make PRs like #121831 mergeable again.

.github/workflows/build.yml Outdated Show resolved Hide resolved
@webknjaz webknjaz force-pushed the bugfixes/gha-job-conditional-flags branch from 911e78f to 84f6498 Compare July 16, 2024 12:15
.github/workflows/build.yml Outdated Show resolved Hide resolved
webknjaz added 2 commits July 16, 2024 14:23
Previously, those flags would sometimes end up having empty string
values, which tends to break evaluating them as JSON. This patch adds
`false` fallbacks to all such outputs.

This allows feeding them to `fromJSON()` without a fear of them
causing surprising internal behaviors in the GitHub Actions CI/CD
workflows platform itself [[1]]. The behavior observed was that
some skipped jobs wouldn't show up in the workflow sidebar view at
all, would display in the graph view as `Waiting for pending jobs`
and in the `${{ needs }}` context, they would have a
`result: failure` entry [[2]].

This should help make PRs like python#121831 mergeable again.

[1]: python#121766 (comment)
[2]: https://github.com/python/cpython/actions/runs/9950331379/job/27501637459?pr=121831#step:2:244
This updates it to avoid referencing to `no-GIL` in favor of the
official term `free-threading`.
@webknjaz webknjaz force-pushed the bugfixes/gha-job-conditional-flags branch from 84f6498 to d42c6f6 Compare July 16, 2024 12:23
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks!

@hugovk hugovk enabled auto-merge (squash) July 16, 2024 12:24
@webknjaz
Copy link
Contributor Author

@hugovk this probably needs backport labels too, right?

@hugovk
Copy link
Member

hugovk commented Jul 16, 2024

Yeah, let's test to confirm it works as expected by updating #121831 after this has merged.

@hugovk hugovk changed the title 🧪🚑 Fix using check_source flags as bool CI: Fix using check_source flags as bool Jul 16, 2024
@hugovk hugovk merged commit a0b205b into python:main Jul 16, 2024
36 checks passed
@hugovk hugovk added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Jul 16, 2024
@miss-islington-app
Copy link

Thanks @webknjaz for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @webknjaz for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @webknjaz and @hugovk, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker a0b205bba555dd9c702b9a856cd9a8153277c9b0 3.12

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 16, 2024
(cherry picked from commit a0b205b)

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk@sydorenko.org.ua>
@bedevere-app
Copy link

bedevere-app bot commented Jul 16, 2024

GH-121853 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 16, 2024
@bedevere-app
Copy link

bedevere-app bot commented Jul 16, 2024

GH-121853 is a backport of this pull request to the 3.13 branch.

webknjaz added a commit to webknjaz/cpython that referenced this pull request Jul 16, 2024
(cherry picked from commit a0b205b)

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk@sydorenko.org.ua>
@bedevere-app
Copy link

bedevere-app bot commented Jul 16, 2024

GH-121855 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Jul 16, 2024
@webknjaz
Copy link
Contributor Author

Manual backport to 3.12: #121855

@bedevere-app
Copy link

bedevere-app bot commented Jul 16, 2024

GH-121855 is a backport of this pull request to the 3.12 branch.

1 similar comment
@bedevere-app
Copy link

bedevere-app bot commented Jul 16, 2024

GH-121855 is a backport of this pull request to the 3.12 branch.

hugovk pushed a commit that referenced this pull request Jul 16, 2024
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk@sydorenko.org.ua>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants