restapi-sidecar: resolve detected branch to the nearest lower supported branch#25671
restapi-sidecar: resolve detected branch to the nearest lower supported branch#25671qiluo-msft wants to merge 1 commit intosonic-net:masterfrom
Conversation
…st supported branch 2. Pre-compile regex patterns at module level to prevent memory leaks from repeated compilation
qiluo-msft
changed the title
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR improves the restapi-sidecar's branch detection and resolution logic to handle all possible SONiC version branches gracefully. Instead of failing when encountering an unsupported branch, it now maps detected branches to the nearest lower supported branch. The PR also optimizes regex pattern compilation by moving it to module level to prevent memory leaks from repeated compilation.
Changes:
- Added
_resolve_branch()function to map any detected branch to the nearest supported version (202311, 202405, 202411, 202505, 202511) - Pre-compiled regex patterns at module level (_MASTER_PATTERN, _INTERNAL_PATTERN, _DATE_PATTERN, _DATE_EXTRACT_PATTERN) to avoid repeated compilation
- Replaced hard-coded branch validation that returned False with flexible resolution logic that always succeeds
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dockers/docker-restapi-sidecar/systemd_stub.py | Adds pre-compiled regex patterns and _resolve_branch() function; updates ensure_sync() to use branch resolution instead of failing on unsupported branches |
| dockers/docker-restapi-sidecar/cli-plugin-tests/test_systemd_stub.py | Adds comprehensive test coverage for _resolve_branch(), regex pattern compilation, and end-to-end integration scenarios |
| if not branch_name.isdigit(): | ||
| logger.log_error(f"Cannot resolve non-numeric branch: {branch_name}, falling back to {SUPPORTED_BRANCHES[0]}") | ||
| return SUPPORTED_BRANCHES[0] | ||
|
|
||
| # String comparison is safe: all YYYYMM values are fixed 6-digit format | ||
| candidates = [b for b in SUPPORTED_BRANCHES if b <= branch_name] |
There was a problem hiding this comment.
The function uses isdigit() to check if branch_name is numeric, but doesn't validate it has exactly 6 digits (YYYYMM format). The comment on line 137 states "all YYYYMM values are fixed 6-digit format" but this isn't enforced. Consider adding validation like if not (branch_name.isdigit() and len(branch_name) == 6) to make the function more robust. While this isn't a practical issue currently (since _get_branch_name() always returns exactly 6 digits for numeric branches), it would prevent incorrect behavior if the function is called with arbitrary input.
| } | ||
|
|
||
|
|
||
| SUPPORTED_BRANCHES = sorted(["202311", "202405", "202411", "202505", "202511"]) |
There was a problem hiding this comment.
The list is already in sorted order, making the sorted() call redundant. Consider either removing sorted() or adding a comment explaining why it's kept (e.g., for clarity or to ensure correctness if the list is modified in the future). Using sorted() here adds a small runtime overhead during module initialization.
| SUPPORTED_BRANCHES = sorted(["202311", "202405", "202411", "202505", "202511"]) | |
| SUPPORTED_BRANCHES = ["202311", "202405", "202411", "202505", "202511"] |
2 participants
So it will handle all possible branches.
Also pre-compile regex patterns at module level to prevent memory leaks from repeated compilation
Why I did it
Work item tracking
How I did it
How to verify it
Unit test
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)