Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Always use a new tmpdir when executing with the process worker #17370

Merged
merged 3 commits into from
Mar 4, 2025

Conversation

cicdw
Copy link
Member

@cicdw cicdw commented Mar 4, 2025

When refactoring the process worker to use a Runner in #17295 we introduced a working directory bug in which, if a working directory was not already provided, we used the working directory from when the worker start command was run.

This is problematic for remote storage, especially git, and so this PR ensures the process worker always creates and manages a temporary working directory per-run.

Closes #17317

@github-actions github-actions bot added the bug Something isn't working label Mar 4, 2025
Copy link

codspeed-hq bot commented Mar 4, 2025

CodSpeed Performance Report

Merging #17370 will not alter performance

Comparing properly-use-tmp-in-process-worker (047a55e) with main (a879067)

Summary

✅ 2 untouched benchmarks

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

nice! TIL nullcontext accepts enter_result

@cicdw cicdw merged commit e1d0c20 into main Mar 4, 2025
46 checks passed
@cicdw cicdw deleted the properly-use-tmp-in-process-worker branch March 4, 2025 22:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
3 participants