Skip to content

Conversation

@ReinierMaas
Copy link
Contributor

@ReinierMaas ReinierMaas commented Dec 24, 2025

Refactor opsqueue_service to use wait_for_server for port assignment and add utility function for server wait logic.

…nment and add utility function for server wait logic
@ReinierMaas ReinierMaas self-assigned this Dec 24, 2025
Copilot AI review requested due to automatic review settings December 24, 2025 15:01
Copy link

Copilot AI left a 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 using psutil to detect when it starts listening on a port
  • Modified opsqueue_service() to use psutil.Popen instead of subprocess.Popen and dynamically discover the server port via wait_for_server()
  • Changed the default port parameter from None to 0, 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.

@ReinierMaas ReinierMaas requested a review from Qqwy December 30, 2025 10:38
class OpsqueueProcess:
port: int
process: subprocess.Popen[bytes]
process: psutil.Popen # subprocess.Popen[bytes]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: old commented code ♻️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants