feat: Add run_subprocess function to the Session class #282
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mwiebe
force-pushed
the
session-run-subprocess
branch
2 times, most recently
from
5780572 to
0c1e13c
Compare
mwiebe
force-pushed
the
session-run-subprocess
branch
2 times, most recently
from
5e4afd8 to
6984f6c
Compare
| This method is non-blocking; it will exit when the subprocess is either | ||
| confirmed to have started running, or has failed to be started. |
There was a problem hiding this comment.
Would it make sense to make blocking an parameter? Maybe not, I'm not quite what the intended use case is.
There was a problem hiding this comment.
This behavior fits run_subprocess into the sequential actions that are part of a session, just like the existing run_task or enter_environment. Expanding the environment to run in parallel to other actions makes sense, but it would need more verification that the code is compatible with that concurrency, compared to fitting in the existing patterns the way it is now. Because of that, I'd rather wait until we have that use case to expand it.
| ValueError: If timeout is provided and is not a positive integer, or if command is empty. | ||
| """ | ||
| # State validation | ||
| if self.state != SessionState.READY: |
There was a problem hiding this comment.
Why not allow RUNNING? If we want to run multiple subprocesses using this new method, it looks like the session would be in RUNNING after the first call and the second call would fail.
There was a problem hiding this comment.
Same thing as for the non-blocking part.
mwiebe
force-pushed
the
session-run-subprocess
branch
from
6984f6c to
548d800
Compare
mwiebe
force-pushed
the
session-run-subprocess
branch
from
548d800 to
2733518
Compare
This new function provides the ability to run an ad hoc command within the session environment, choosing whether to use the environment variables from the session enter actions or to ignore them. The parameters for the command are direct, so you don't have to construct a temporary Action or StepAction object to run a simple command. Signed-off-by: Mark <[email protected]>
mwiebe
force-pushed
the
session-run-subprocess
branch
from
2733518 to
1e6c05b
Compare
|
3 participants
What was the problem/requirement? (What/Why)
We need the ability to run subprocesses that run the same way as all the action subprocesses do, but are not actions specified in the job template. This includes the need to control whether they inherit the full session environment including envEnter variables or not.
The proposed change in #281 solves the problem, but introduces a new parameter to
run_taskthat doesn't fit within the spirit of the OpenJD Task, which is a specific entity within the specification.This change was created using Kiro with the Claude Sonnet 4.5 model.
What was the solution? (How)
This new function provides the ability to run an ad hoc command within the session environment, choosing whether to use the environment variables from the session enter actions or to ignore them. The parameters for the command are direct, so you don't have to construct a temporary Action or StepAction object to run a simple command.
What is the impact of this change?
The Session object is more flexible to use in custom use cases.
How was this change tested?
Implemented unit tests for the new functionality.
Was this change documented?
Added a docstring.
Is this a breaking change?
No
Does this change impact security?
No
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.