[launch] consider refactoring ExecuteProcess into Execute and Executable
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 155
- Forks
- 182
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 6
Description
I believe it might be wise to separate the description of a process to be executed and its state while running from the Action which executes it.
Currently all of this information is encapsulated in the ExecuteProcess action, and my proposal would be to replace that with an action called Execute that takes an Executable (not an action or launch description entity, just a new kind of class). The Executable would contain any information needed by Execute to actually execute the executable described or introspect it. There could be sub classes of both Execute and Executable. For instance, a sub class of Executable might be Node or LifecycleNode, and a sub class of Execute might be ExecuteRemotely (execute on a different machine).
The reasons for doing this include:
- using
Executables (or subclasses thereof) with other actions - cleaner abstractions
- i.e. the
ExecuteProcessclass is really big now and it would be nice to break into smaller parts
- i.e. the
- more readable
- e.g.
Execute(Executable(...))orExecute(Node(...))rather thanExecuteProcess(...)andNode(...)
- e.g.
The first reason is the most impactful, because it allows for a few use cases that aren't currently easy to implement:
- Collect or generate
Executableinstances and later decide whether to pass toExecute,ExecuteRemotely, orExecuteInDocker(just ideas) - Pass
ComponentNode(possibly a subclass ofExecutablewhich represents a node that can be loaded dynamically) to eitherExecute()orExecuteNodesInSharedProcess()(name needs work)- where the latter would take several
ComponentNodeinstances and run them all in a single process
- where the latter would take several
I still have some design questions in mind, like:
- should
Nodeinherit fromExecutable - should
Executablecontain process related things likepidand produce events for stdout and process exit, etc...- or should there be a
Processthat also inherits fromExecutable - because
Nodemight not be associated with a process, or rather a process might not be associated with a singleNode
- or should there be a
- what is the contract between
ExecutableandExecute?- which creates event handlers and stores information?
- what exactly are the responsibilities of
Execute?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the existing ExecuteProcess action and its uses in the launch codebase. Work through the proposed Execute and Executable responsibilities, including Node, process state, events, and remote execution. Done requires an agreed design for the abstractions and a coordinated implementation plan.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100