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

Update globus_compute.py #3765

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update globus_compute.py #3765

wants to merge 3 commits into from

Conversation

noelppp
Copy link
Contributor

@noelppp noelppp commented Feb 12, 2025

Add two arguments storage_access and working_dir.
@yadudoc

Description

These arguments are needed to allow custom staging providers.
https://parsl.readthedocs.io/en/stable/userguide/configuration/data.html

Type of change

  • Code maintenance/cleanup

Add two arguments `storage_access` and `working_dir`.
Copy link
Member

@yadudoc yadudoc left a comment

Choose a reason for hiding this comment

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

Hi @noelppp, thanks for taking the time to push the PR.
Adding the storage_access attribute makes sense.

I'm concerned about working_dir though. If working_dir were set here I would expect all functions submitted to run in that directory, while I do not see any calls to the GCExecutor to set this up. Have you tested working_dir by itself?

@noelppp
Copy link
Contributor Author

noelppp commented Feb 14, 2025

That's a good point.

Have you tested working_dir by itself?

No, but without working_dir I got an error because some staging providers require it
https://github.com/search?q=repo%3AParsl%2Fparsl%20dm.dfk.executors%5Bexecutor%5D.working_dir&type=code

@benclifford
Copy link
Collaborator

@yadudoc that's not what working_dir means in other executors -- it's only used for file staging placement.

@yadudoc
Copy link
Member

yadudoc commented Feb 20, 2025

@noelppp, I'm so sorry! My response on this PR sat unsent for a week!

I see how working_dir in this context is only related to staging. I'd recommend adding a line documenting these two new arguments to avoid confusion with the working_dir that could be set on the globus-compute-endpoint configurations.

@noelppp
Copy link
Contributor Author

noelppp commented Feb 20, 2025

@yadudoc No worries!

I added some documentation for the two new arguments

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.

3 participants