Fixed the issue #5550.Supports specifying port ranges, with automatic matching between listen and URI.#6280
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the flexibility of the status server's network configuration by introducing support for port ranges. Users can now specify a range of ports, and the system will intelligently select an available port within that range for both the server to listen on and for clients to connect to. This change addresses potential port conflicts and simplifies deployment in dynamic environments, ensuring a more robust and adaptable status server setup. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a useful feature to support port ranges for the status server, which includes automatic matching between the listen and URI configurations. The implementation correctly finds an available port and updates the configuration for other components. However, my review identified a few areas for improvement. There is significant code duplication in parsing port ranges across multiple files, which would benefit from being refactored into a shared utility function. I also found duplicated logic for handling single-port versus port-range connections that could be simplified. More critically, there appears to be dead code for port range handling in StatusServer.create_server, as the port is resolved before the server is instantiated. Lastly, I've noted a minor typo in the documentation.
avocado/core/status/server.py
Outdated
| if "-" in port_spec: | ||
| start_s, end_s = port_spec.split("-", 1) | ||
| try: | ||
| start = int(start_s) | ||
| end = int(end_s) | ||
| except ValueError as exc: | ||
| raise ValueError( | ||
| f"Invalid port range in status server URI: {self._uri}" | ||
| ) from exc | ||
| if start > end: | ||
| raise ValueError( | ||
| f"Invalid port range (start > end) in status server URI: {self._uri}" | ||
| ) | ||
| last_exc = None | ||
| for port in range(start, end + 1): | ||
| try: | ||
| self._server_task = await asyncio.start_server( | ||
| self.cb, host=host, port=port, limit=limit | ||
| ) | ||
| # Update URI to the actual chosen port so that | ||
| # other components can rely on the concrete value. | ||
| self._uri = f"{host}:{port}" | ||
| LOG_JOB.info("Status server listening on %s", self._uri) | ||
| break | ||
| except OSError as exc: | ||
| last_exc = exc | ||
| if self._server_task is None: | ||
| # Re-raise the last failure or a generic error if none. | ||
| raise last_exc or OSError( | ||
| f"Could not bind status server to any port in range {start}-{end} on {host}" | ||
| ) |
There was a problem hiding this comment.
The logic for handling port ranges in create_server appears to be dead code. The _resolve_listen_address method in avocado/plugins/runner_nrunner.py already finds an available port from the range and resolves it to a concrete host:port address before StatusServer is instantiated. Therefore, StatusServer will always receive a URI with a single port, and this port range handling logic will never be executed. This adds unnecessary complexity and should be removed.
avocado/core/nrunner/task.py
Outdated
| if "-" in port_spec: | ||
| start_s, end_s = port_spec.split("-", 1) | ||
| try: | ||
| self._connection = socket.create_connection((host, port)) | ||
| break | ||
| except ConnectionRefusedError as error: | ||
| LOG.warning(error) | ||
| time.sleep(1) | ||
| start = int(start_s) | ||
| end = int(end_s) | ||
| except ValueError as exc: | ||
| raise ValueError( | ||
| f"Invalid port range in status server URI: {self.uri}" | ||
| ) from exc | ||
| if start > end: | ||
| raise ValueError( | ||
| f"Invalid port range (start > end) in status server URI: {self.uri}" | ||
| ) | ||
| last_error = None | ||
| for port in range(start, end + 1): | ||
| for _ in range(600): | ||
| try: | ||
| self._connection = socket.create_connection((host, port)) | ||
| break | ||
| except ConnectionRefusedError as error: | ||
| last_error = error | ||
| LOG.warning(error) | ||
| time.sleep(1) | ||
| if self._connection is not None: | ||
| break | ||
| else: | ||
| # If we could not connect to any port in the range even | ||
| # after retries, raise the last connection error (if any) | ||
| # or attempt one final connection to the last port so that | ||
| # the underlying exception is surfaced. | ||
| if last_error is not None: | ||
| raise last_error | ||
| self._connection = socket.create_connection((host, end)) | ||
| else: | ||
| self._connection = socket.create_connection((host, port)) | ||
| port = int(port_spec) | ||
| for _ in range(600): | ||
| try: | ||
| self._connection = socket.create_connection((host, port)) | ||
| break | ||
| except ConnectionRefusedError as error: | ||
| LOG.warning(error) | ||
| time.sleep(1) | ||
| else: | ||
| self._connection = socket.create_connection((host, port)) |
There was a problem hiding this comment.
There is significant code duplication between the logic for handling a port range (lines 57-89) and a single port (lines 90-100). Both branches contain a retry loop for socket.create_connection. This can be refactored into a single piece of logic that iterates over a list of ports, which would contain either one or multiple ports. This would improve maintainability and readability.
ports = []
if "-" in port_spec:
start_s, end_s = port_spec.split("-", 1)
try:
start = int(start_s)
end = int(end_s)
except ValueError as exc:
raise ValueError(
f"Invalid port range in status server URI: {self.uri}"
) from exc
if start > end:
raise ValueError(
f"Invalid port range (start > end) in status server URI: {self.uri}"
)
ports = range(start, end + 1)
else:
try:
ports = [int(port_spec)]
except ValueError as exc:
raise ValueError(
f"Invalid port in status server URI: {self.uri}"
) from exc
last_error = None
last_port_tried = None
for port in ports:
last_port_tried = port
for _ in range(600):
try:
self._connection = socket.create_connection((host, port))
break
except ConnectionRefusedError as error:
last_error = error
LOG.warning(error)
time.sleep(1)
if self._connection is not None:
break
else:
if last_error is not None:
raise last_error
# If we could not connect, attempt one final connection to the
# last port so that the underlying exception is surfaced.
if last_port_tried is not None:
self._connection = socket.create_connection((host, last_port_tried))
avocado/plugins/runner_nrunner.py
Outdated
| start_s, end_s = port_spec.split("-", 1) | ||
| try: | ||
| start = int(start_s) | ||
| end = int(end_s) | ||
| except ValueError as exc: | ||
| raise JobError( | ||
| f"Invalid port range in status server URI: {listen}" | ||
| ) from exc | ||
| if start > end: | ||
| raise JobError( | ||
| f"Invalid port range (start > end) in status server URI: {listen}" | ||
| ) |
There was a problem hiding this comment.
The logic for parsing a port range from a URI string is duplicated across multiple files: here in _resolve_listen_address, in avocado/core/nrunner/task.py (_create_connection), and in avocado/core/status/server.py (create_server). This should be extracted into a common utility function to avoid code duplication and improve maintainability.
man/avocado.rst
Outdated
| chosen. This is only effective if | ||
| "status_server_auto" is disabled. If | ||
| "status_server_uri" is not set, the value from | ||
| "status_server_listen " will be used. |
There was a problem hiding this comment.
There's a typo here. "status_server_listen " has a trailing space. It should be "status_server_listen".
| "status_server_listen " will be used. | |
| "status_server_listen" will be used. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6280 +/- ##
==========================================
+ Coverage 73.48% 73.65% +0.17%
==========================================
Files 206 206
Lines 22494 22655 +161
==========================================
+ Hits 16530 16687 +157
- Misses 5964 5968 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: xianglongfei <xianglongfei@uniontech.com>
github-project-automation
bot
moved this from Review Requested
to Done 114
in Default project
1 participant
Fixed the issue #5550.Supports specifying port ranges, with automatic matching between listen and URI.
Command line:
avocado run examples/tests/passtest.py --status-server-disable-auto --status-server-listen 127.0.0.1:8888-9000 --status-server-uri 127.0.0.1:8888-9000
Use the testing program to occupy ports 8888/8889 for verification
logs:
...
2026-03-04 15:06:58,073 avocado.job job L0291 INFO | 'run.status_server_listen': '127.0.0.1:8888-9000',
2026-03-04 15:06:58,073 avocado.job job L0291 INFO | 'run.status_server_uri': '127.0.0.1:8888-9000',
...
2026-03-04 15:06:58,439 avocado.job server L0068 INFO | Status server listening on 127.0.0.1:8890
2026-03-04 15:06:58,740 avocado.job testlogs L0132 INFO | examples/tests/passtest.py:PassTest.test: STARTED
2026-03-04 15:06:58,948 avocado.job testlogs L0138 INFO | examples/tests/passtest.py:PassTest.test: PASS
...
Signed-off-by: xianglongfei xianglongfei@uniontech.com