Skip to content

Expanded Python test + faster caching#687

Open
jaybosamiya-ms wants to merge 2 commits intomainfrom
jayb/python3121-fixup
Open

Expanded Python test + faster caching#687
jaybosamiya-ms wants to merge 2 commits intomainfrom
jayb/python3121-fixup

Conversation

@jaybosamiya-ms
Copy link
Member

@jaybosamiya-ms jaybosamiya-ms commented Feb 26, 2026

This PR has two somewhat related changes I hit when running the Python test. Some versions of Python (concretely, 3.12.1) require more files to be pulled in than were being pulled in before; the first commit in this PR fixes this. The second commit speeds the cache up by just using mtime+size rather than spending a bunch of time on computing sha256.

@jaybosamiya-ms jaybosamiya-ms marked this pull request as ready for review February 26, 2026 03:26
@jaybosamiya-ms
Copy link
Member Author

jaybosamiya-ms commented Feb 26, 2026

Note: the sha256 change shifts one bottleneck to further along the pipeline for the particular environment I am running this in, but I will leave those changes to a separate PR when I get some more cycles.

@github-actions
Copy link

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

Copy link
Member

@wdcui wdcui left a comment

Choose a reason for hiding this comment

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

LGTM. I left one comment below.

.duration_since(std::time::UNIX_EPOCH)
.unwrap_or_default();
let size = metadata.len();
Ok(format!("{}:{}", mtime.as_nanos(), size))
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to include the file path in the summary?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, does not really hurt to include the file path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants