Add release branch automation script for TheRock and ROCm submodules#52
Conversation
|
moved code from #36 to this new PR. Since author of that is not available in this project. Hi @araravik-psd, your last comment was addressed in the PR #36 , please review and let me know if any new changes are required. |
araravik-psd
left a comment
araravik-psd
left a comment
There was a problem hiding this comment.
HI @amd-chiranjeevi a few more comments that might be needed, please take a look
araravik-psd
left a comment
araravik-psd
left a comment
There was a problem hiding this comment.
Hi @amd-chiranjeevi, changes look good, please take a look at the below exception handling cases. Do we have a test run for these changes?
| continue | ||
|
|
||
| try: | ||
| branch_exists = self._remote_branch_exists(info.path) |
There was a problem hiding this comment.
execute_plan only catches CalledProcessError, not subprocess.TimeoutExpired. A network timeout on ls-remote or push will crash the entire run instead of failing just that repo. Add TimeoutExpired to every except block in execute_plan
Add a except that catches both exceptions in the two blocks that do network I/O:
try:
branch_exists = self._remote_branch_exists(info.path)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as exc:
failed_repos[repo_name] = (
f"Remote branch check failed: {exc}"
)
continue
| continue | ||
|
|
||
| try: | ||
| self._push_branch(repo_name, info.path) |
There was a problem hiding this comment.
Similar changes as above here to handle exception
try:
self._push_branch(repo_name, info.path)
successful_repos[repo_name] = info
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as exc:
failed_repos[repo_name] = f"Branch push failed: {exc}"
There was a problem hiding this comment.
Thanks changes done.
| level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s" | ||
| ) | ||
|
|
||
| RockBranchingAutomation(args).run() |
There was a problem hiding this comment.
main() swallows exceptions here, errors produce a raw traceback instead of a clean exit
RockBranchingAutomation(args).run() can raise RuntimeError (from cache validation in def build_plan()), subprocess.CalledProcessError, or subprocess.TimeoutExpired. Any of these crash with a raw Python traceback rather than a clean message. Wrap it with a try except loop:
try:
RockBranchingAutomation(args).run()
return 0
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as exc:
logging.error("Command failed: %s", exc)
return 1
except RuntimeError as exc:
logging.error("%s", exc)
return 1
except Exception as exc:
logging.error("Unexpected error: %s", exc)
return 1
There was a problem hiding this comment.
Thanks. changes done.
yes Aravind, I have tested and created in repos conformed by checking the repos ex: https://github.com/ROCm/rocm-cmake/commits/users/amd-chiranjeevi/test_branch_2/ , https://github.com/ROCm/half/tree/users/amd-chiranjeevi/test_branch_2 |
|
Unit tests execution: |
araravik-psd
left a comment
araravik-psd
left a comment
There was a problem hiding this comment.
@amd-chiranjeevi thanks for addressing the review comments
2 participants
Motivation
TheRock's release process requires creating release branches across TheRock and all its tracked ROCm submodules at a specific commit. Doing this manually is error-prone and time-consuming given the number of submodules involved. This script automates the entire workflow end-to-end.
Technical Details
Test Plan
Test Result