fix(verl): load Kubernetes job templates as UTF-8 - #635
Open
Bluu (Bluuok) wants to merge 1 commit into
Open
Bluu (Bluuok) wants to merge 1 commit into
Bluu (Bluuok) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused encoding fix is correct and covered by an appropriate regression test.
Review effort: Balanced
Findings: None
What changed in this PR
Ensures Kubernetes Job templates load consistently as UTF-8, including on Windows systems using GBK locales.
Changes:
- Reads Job templates with explicit UTF-8 encoding.
- Adds a regression test preserving non-ASCII template text.
| File | Description |
|---|---|
agentlightning/verl/agl_rollout_manager.py |
Explicitly decodes Kubernetes templates as UTF-8. |
tests/verl/test_agl_rollout_manager.py |
Tests UTF-8 loading and client cleanup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A valid UTF-8 Kubernetes Job template can fail during rollout-manager initialization on Windows with a GBK default locale.
Path.read_text()uses that locale and raisesUnicodeDecodeErrorfor ordinary UTF-8 text, before rollouts can be queued.Read the template explicitly as UTF-8. Add a real manager-initialization regression with an em dash in the template and assert the complete text retained in the rollout configuration; close the allocated client afterwards.
Validation with the frozen native Windows Python 3.12 environment:
Unchanged main: the regression failed with the actual GBK
UnicodeDecodeError(byte 0x94). With the fix: 1 target test and all 14 tests in the file passed. Scoped Pyright passed with 0 errors. This regression is platform-specific: systems whose default locale is already UTF-8 can pass before the fix.No HTTP calls, model inference or GPU training were run. Ready for review based on the author-reported validation above; this maintenance pass did not rerun tests. The CLA check passed. Fork CI is awaiting maintainer approval (action_required, 0 jobs); no CI jobs have run. AI assistance was used; the one-line production change and regression were reviewed.