chore(ci): fix header comment loss in update_overrides on repeated runs#503
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the update_overrides.py helper to keep the pyproject.toml “Security overrides” header stable when regenerating override-dependencies, and applies small formatting cleanups.
Changes:
- Reformats long
print(...)andre.sub(...)calls for readability. - Removes existing
override-dependenciesblocks and associated header comments. - Re-inserts a standardized “Security overrides” header after
[tool.uv]to support consistent insertion behavior.
|
Hi @Fiona-Waters! Could you review this PR? |
Fiona-Waters
left a comment
There was a problem hiding this comment.
Thanks for this @yogarajalakshmi-s
Looks good. Should we include \r here too?
Signed-off-by: Yogarajalakshmi S <yogarajalakshmis@gmail.com>
Signed-off-by: Yogarajalakshmi S <yogarajalakshmis@gmail.com>
Signed-off-by: Yogarajalakshmi S <yogarajalakshmis@gmail.com>
8147af6 to
fecc825
Compare
Yes! Updated both insertion points to use \r |
Fiona-Waters
left a comment
There was a problem hiding this comment.
Thanks @yogarajalakshmi-s
/lgtm
What this PR does / why we need it:
When has_overrides=True, update_overrides.py removes the "# Security overrides" header comment but never re-adds it. On the next run, the insertion logic uses this header as a bookmark to know where to insert the override block — but since it was deleted, the block gets inserted incorrectly. Fixed by reinserting the header after [tool.uv] following the cleanup step.
This bug was discovered while adapting this script for kubeflow/trainer in kubeflow/trainer#3539.
Which issue(s) this PR fixes:
Fixes #502
Checklist: