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

Add a run_in_background option for executables #573

Merged
merged 3 commits into from
Jul 21, 2024

Conversation

linsword13
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@douglasjacobsen douglasjacobsen left a comment

Choose a reason for hiding this comment

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

One small question.

As an aside, is it useful to capture the PID of the backgrounded process, so we can do something like sleep until it's finished?

@linsword13
Copy link
Collaborator Author

One small question.

As an aside, is it useful to capture the PID of the backgrounded process, so we can do something like sleep until it's finished?

Yeah good call. I was thinking to have a modifier to achieve that (using an executable_modifier to capture the PID,) and then do a wait-for-all at the end. I feel that might be more flexible. Will add such a modifier in this PR.

@linsword13
Copy link
Collaborator Author

One small question.
As an aside, is it useful to capture the PID of the backgrounded process, so we can do something like sleep until it's finished?

Yeah good call. I was thinking to have a modifier to achieve that (using an executable_modifier to capture the PID,) and then do a wait-for-all at the end. I feel that might be more flexible. Will add such a modifier in this PR.

Added a modifier doing the wait-for-completion for background jobs.

Copy link
Collaborator

@douglasjacobsen douglasjacobsen left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes! This is awesome.

One small comment, otherwise it LGTM. Feel free to choose between adding the conditional check or adding a comment in the doc string.

@douglasjacobsen douglasjacobsen merged commit 635e4d3 into GoogleCloudPlatform:develop Jul 21, 2024
11 checks passed
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