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

Enable working_dir configurability and safety #1689

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

yadudoc
Copy link
Collaborator

@yadudoc yadudoc commented Oct 17, 2024

Description

Currently ThreadPoolEngine and ProcessPoolEngine now pass the working_dir: default tasks_working_dir.
Executing functions on these engines affect subsequent function execution when they say change directory for eg, unlike GlobusComputeEngine. When executing functions, our execute_task wrapper currently attempts to look for a tasks_working_dir in the current dir, creates it if not present, and then switches into it. Unfortunately, each subsequent function finds itself one directory deeper.

  • This PR updates GlobusComputeEngineBase to check if the working_dir is an absolute path, and if not make it absolute. This ensures that every function would run in ~/.globus_compute/<endpoint_name>/tasks_working_dir.
  • working_dir is now a configurable option for all engines.
  • tests to confirm that running multiple functions do not change directory

Fixes # (issue)

[sc-36710]

Type of change

Choose which options apply, and delete the ones which do not apply.

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation update
  • Code maintenance/cleanup

@yadudoc yadudoc force-pushed the configure_tasks_working_dir branch from 4891e3b to c34d27b Compare October 23, 2024 15:10
@yadudoc yadudoc added the quick-review Review of this should be quick and easy label Oct 23, 2024
@yadudoc yadudoc requested review from khk-globus and rjmello October 23, 2024 19:20
@yadudoc yadudoc force-pushed the configure_tasks_working_dir branch 3 times, most recently from 28c9e5e to 22cc988 Compare October 31, 2024 14:05
@khk-globus khk-globus removed the quick-review Review of this should be quick and easy label Nov 3, 2024
@yadudoc yadudoc force-pushed the configure_tasks_working_dir branch 2 times, most recently from cec12b8 to 2223771 Compare November 11, 2024 17:51
@yadudoc yadudoc force-pushed the configure_tasks_working_dir branch 4 times, most recently from f88331a to dcdec87 Compare November 13, 2024 00:56
`ThreadPoolEngine` and `ProcessPoolEngine` now pass the `working_dir` default
`"tasks_working_dir"` to the `GlobusComputeEngineBase`, which makes the path
absolute.  When `execute_task` runs the cwd is changed to `working_dir`,
ensuring that tasks run in the same directory.

* Added a new method `GlobusComputeEngineBase.set_working_dir()` to
  ensure `working_dir` is absolute.

* Enforce absolute paths for `run_dir` in `execute_task()`

* Removed an obsolete test
@khk-globus khk-globus force-pushed the configure_tasks_working_dir branch from dcdec87 to 6865316 Compare November 13, 2024 02:45
@khk-globus khk-globus merged commit 1cd2f0d into main Nov 13, 2024
17 checks passed
@khk-globus khk-globus deleted the configure_tasks_working_dir branch November 13, 2024 02:51
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