-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor opsqueue_service to use wait_for_server for port assignment and add utility function for server wait logic #34
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
base: reinier/log-bound-server-port
Are you sure you want to change the base?
Conversation
…nment and add utility function for server wait logic
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 refactors the test infrastructure to dynamically detect server ports rather than pre-assigning them. The main change introduces a wait_for_server utility function that monitors a process until it starts listening on a port, eliminating the need for the previous random_free_port() approach.
Key Changes:
- Added a new
wait_for_server()utility function that polls a process usingpsutilto detect when it starts listening on a port - Modified
opsqueue_service()to usepsutil.Popeninstead ofsubprocess.Popenand dynamically discover the server port viawait_for_server() - Changed the default port parameter from
Noneto0, which allows the OS to assign an available port that is then discovered
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| libs/opsqueue_python/tests/util.py | New utility module containing wait_for_server() function with retry logic to wait for a process to start listening on a port |
| libs/opsqueue_python/tests/conftest.py | Updated opsqueue_service() to use psutil.Popen and call wait_for_server() for dynamic port discovery instead of pre-assigning ports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…` assignment and add utility function for server wait logic
| class OpsqueueProcess: | ||
| port: int | ||
| process: subprocess.Popen[bytes] | ||
| process: psutil.Popen # subprocess.Popen[bytes] |
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.
Nit: old commented code ♻️
Refactor
opsqueue_serviceto usewait_for_serverfor port assignment and add utility function for server wait logic.