ros2 / ros2/launch

[launch] consider refactoring ExecuteProcess into Execute and Executable

Open
#114 7 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backlog enhancement
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 ExecuteProcess class is really big now and it would be nice to break into smaller parts
  • more readable
    • e.g. Execute(Executable(...)) or Execute(Node(...)) rather than ExecuteProcess(...) and Node(...)

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 Executable instances and later decide whether to pass to Execute, ExecuteRemotely, or ExecuteInDocker (just ideas)
  • Pass ComponentNode (possibly a subclass of Executable which represents a node that can be loaded dynamically) to either Execute() or ExecuteNodesInSharedProcess() (name needs work)
    • where the latter would take several ComponentNode instances and run them all in a single process

I still have some design questions in mind, like:

  • should Node inherit from Executable
  • should Executable contain process related things like pid and produce events for stdout and process exit, etc...
    • or should there be a Process that also inherits from Executable
    • because Node might not be associated with a process, or rather a process might not be associated with a single Node
  • what is the contract between Executable and Execute?
    • which creates event handlers and stores information?
    • what exactly are the responsibilities of Execute?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.