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 execute_task signature #1719

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Conversation

khk-globus
Copy link
Contributor

run_dir is not optional; in a previous commit, it was enforced by an inline check. Update the function signature to match that requirement.

While here, test the addition, and also bolster the module's tests in general.

Type of change

  • Code maintenance/cleanup

@khk-globus khk-globus added the no-news-is-good-news This change does not require a news file label Nov 13, 2024
@khk-globus khk-globus changed the title Update execute_task signature (#1719) Update execute_task signature Nov 13, 2024
Copy link
Collaborator

@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.

I am not clear on the directory handling around sandboxing, so I've left a comment there. The tests look good.

khk-globus added a commit that referenced this pull request Nov 13, 2024
`run_dir` is not optional; in a previous commit, it was enforced by an inline
check.  Update the function signature to match that requirement.

While here, test the addition, and also bolster the module's tests in general.
@khk-globus khk-globus force-pushed the update_execute_task_signature branch from d37e03f to fbd60e2 Compare November 13, 2024 21:23
@khk-globus khk-globus requested a review from yadudoc November 13, 2024 21:23
`run_dir` is not optional; in a previous commit, it was enforced by an inline
check.  Update the function signature to match that requirement.

While here, test the addition, and also bolster the module's tests in general.
@khk-globus khk-globus force-pushed the update_execute_task_signature branch from fbd60e2 to 6fcdc08 Compare November 14, 2024 14:08
Copy link
Collaborator

@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.

Nice!

@khk-globus khk-globus merged commit 55e0759 into main Nov 14, 2024
17 checks passed
@khk-globus khk-globus deleted the update_execute_task_signature branch November 14, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-news-is-good-news This change does not require a news file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants