Skip to content

Modernize esrally/utils/process.py#2147

Open
pquentin wants to merge 3 commits into
elastic:masterfrom
pquentin:refactor-subprocess-calls
Open

Modernize esrally/utils/process.py#2147
pquentin wants to merge 3 commits into
elastic:masterfrom
pquentin:refactor-subprocess-calls

Conversation

@pquentin

Copy link
Copy Markdown
Member

The next question is: why do we have four different ways to run subprocesses? Do we even need those helpers now that subprocess.run() exists?

Copilot AI review requested due to automatic review settings June 16, 2026 06:41
@pquentin pquentin requested a review from a team as a code owner June 16, 2026 06:41
@pquentin pquentin added cleanup Linter changes, reformatting, removal of unused code etc. :internal Changes for internal, undocumented features: e.g. experimental, release scripts labels Jun 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes Rally’s subprocess helpers in esrally/utils/process.py by migrating legacy subprocess.call() / Popen() usage to subprocess.run() and replacing preexec_fn-based detaching with start_new_session.

Changes:

  • Switch run_subprocess() and run_subprocess_with_output() to subprocess.run().
  • Replace preexec_fn=os.setpgrp with start_new_session=detach for detaching.
  • Simplify output capture by relying on text=True and processing CompletedProcess.stdout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread esrally/utils/process.py
Comment thread esrally/utils/process.py
Comment thread esrally/utils/process.py
Comment thread esrally/utils/process.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Linter changes, reformatting, removal of unused code etc. :internal Changes for internal, undocumented features: e.g. experimental, release scripts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants