-
Notifications
You must be signed in to change notification settings - Fork 320
Allow configuring the process startup method to be used for creating thespian actors. #1975
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
Conversation
The main reason for this is that to be able to use threads in subprocesses we have to prevemt using any kind of fork function.
There was a problem hiding this comment.
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 modifies the Rally actor system to use spawn instead of fork for subprocess creation, enabling thread usage in subprocesses. The main changes include:
- Adding process startup method configuration and validation
- Enhanced actor system detection and bootstrapping logic
- Comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| esrally/actor.py | Core implementation adding process startup method support, improved type annotations, and enhanced actor system detection |
| esrally/rally.py | Integration of process startup method configuration validation and actor system setup |
| esrally/types.py | Added new configuration keys for actor process startup method |
| tests/actor_test.py | Comprehensive test suite covering all actor system bootstrap scenarios and configurations |
| pyproject.toml | Added mypy override for the actor module |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
gbanasiak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I've left some minor comments and questions, but nothing blocking.
…reating thespian actors. (elastic#1975)" This reverts commit 81abfc3.
The main purpose for this is to be able to prevent using the fork function to be able to use threads for multipart downloads.