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.

Sign up for GitHub

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 more ubuntu-24.04 failures #4610

Merged
merged 3 commits into from
Oct 10, 2024
Merged

ci: fix more ubuntu-24.04 failures #4610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f465be
ci: fix more ubuntu-24.04 failures
davidhewitt Oct 10, 2024
787c0ba
use 22.04 to test 3.7
davidhewitt Oct 10, 2024
c56fa45
disable zoneinfo test on free-threading
davidhewitt Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,15 @@ jobs:
python-architecture: "x64",
rust-target: "x86_64-apple-darwin",
}
# ubuntu-latest (24.04) no longer supports 3.7, so run on 22.04
- rust: stable
python-version: "3.7"
platform:
{
os: "ubuntu-22.04",
python-architecture: "x64",
rust-target: "x86_64-unknown-linux-gnu",
}

# arm64 macOS Python not available on GitHub Actions until 3.10
# so backfill 3.7-3.9 with x64 macOS runners
Expand Down Expand Up @@ -341,6 +350,9 @@ jobs:
}

exclude:
# ubuntu-latest (24.04) no longer supports 3.7
- python-version: "3.7"
platform: { os: "ubuntu-latest" }
# arm64 macOS Python not available on GitHub Actions until 3.10
- rust: stable
python-version: "3.7"
Expand Down Expand Up @@ -572,6 +584,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.12'
- uses: Swatinem/rust-cache@v2
with:
save-if: ${{ github.event_name != 'merge_group' }}
Expand Down Expand Up @@ -641,6 +656,9 @@ jobs:
target: "x86_64-apple-darwin"
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.12'
- uses: Swatinem/rust-cache@v2
with:
workspaces:
Expand Down
1 change: 1 addition & 0 deletions src/conversions/chrono_tz.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ mod tests {
}

#[test]
#[cfg(not(Py_GIL_DISABLED))] // https://github.com/python/cpython/issues/116738#issuecomment-2404360445
Copy link
Member Author

Choose a reason for hiding this comment

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

We keep having a flaky failure on this test, from a quick look it seems to me that ZoneInfo is not thread-safe. cc @ngoldbaum

I've included here because CI is a bit deadlocked between this failure and the other fixes in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the heads up! I'll try to take a look at this and report upstream to CPython if the bug is there. Disabling the test on the free-threaded build makes sense if it's flaky.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can reproduce this as well as some other failures I haven't seen until now.

When I switch back to rc2, I don't see them, so there must have been a CPython change between rc2 and final that caused this to happen.

ping @colesbury since this looks like it's happening because of late changes in CPython right before the release.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh no, I can reproduce it on rc2. I think I missed this on my prior passes over the tests because I wasn't passing --features=full in my cargo test command. Sorry for the noise Sam!

fn test_into_pyobject() {
Python::with_gil(|py| {
let assert_eq = |l: Bound<'_, PyAny>, r: Bound<'_, PyAny>| {
Expand Down
Loading