Skip to content

[fix] Bring back pretty log formatting#250

Merged
SumanthRH merged 4 commits intoNovaSky-AI:mainfrom
tyler-griggs:tgriggs/ray_log
Sep 8, 2025
Merged

[fix] Bring back pretty log formatting#250
SumanthRH merged 4 commits intoNovaSky-AI:mainfrom
tyler-griggs:tgriggs/ray_log

Conversation

@tyler-griggs
Copy link
Member

@tyler-griggs tyler-griggs commented Sep 7, 2025

Brings back nicely-formatted logs (ie, coloring, bold, etc.) that was lost when the training loop moved off the head node.

Old:
Screenshot 2025-09-07 at 12 52 00 PM

New:
Screenshot 2025-09-07 at 4 30 04 PM

@tyler-griggs tyler-griggs changed the title [fix] [fix] Bring bag log formatting Sep 7, 2025
@tyler-griggs tyler-griggs marked this pull request as ready for review September 7, 2025 23:43
@tyler-griggs tyler-griggs changed the title [fix] Bring bag log formatting [fix] Bring back pretty log formatting Sep 7, 2025
Comment on lines +501 to +502
logging.root.handlers = [_InterceptHandler()]
logging.root.setLevel(logging.INFO)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm if I'm debugging and I set LOG_LEVEL=DEBUG in my env file, this will override the root logger to use INFO?
Maybe set it only if LOG_LEVEL is not set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call!

@SumanthRH SumanthRH merged commit 5d9888d into NovaSky-AI:main Sep 8, 2025
3 checks passed
CharlieFRuan added a commit that referenced this pull request Sep 8, 2025
CharlieFRuan added a commit that referenced this pull request Sep 8, 2025
Reverts #250

Will run into ` AssertionError: The env var,
__RAY_WORKER_PROCESS_SETUP_HOOK_ENV_VAR, is not permitted because it is
reserved for the internal use.`
tyler-griggs added a commit that referenced this pull request Sep 11, 2025
Re-implement the logging fix of #250 that was reverted in #261

The issue was that using the `worker_process_setup_hook` to set logging
behavior interfered with vLLM using Ray as it's tensor parallel backend
and threw an error. vLLM apparently needs this to be unset.

Moved the logging configuration into RayPPOTrainer `init`.
ztcanddota added a commit to ztcanddota/skyagent that referenced this pull request Sep 28, 2025
Reverts NovaSky-AI/SkyRL#250

Will run into ` AssertionError: The env var,
__RAY_WORKER_PROCESS_SETUP_HOOK_ENV_VAR, is not permitted because it is
reserved for the internal use.`
SungjunlaLee added a commit to SungjunlaLee/SkyRL that referenced this pull request Jan 3, 2026
Reverts NovaSky-AI/SkyRL#250

Will run into ` AssertionError: The env var,
__RAY_WORKER_PROCESS_SETUP_HOOK_ENV_VAR, is not permitted because it is
reserved for the internal use.`
dzorlu referenced this pull request in fleet-ai/SkyRL Feb 4, 2026
Brings back nicely-formatted logs (ie, coloring, bold, etc.) that was
lost when the training loop moved off the head node.

Old:
<img width="1210" height="196" alt="Screenshot 2025-09-07 at 12 52
00 PM"
src="https://github.com/user-attachments/assets/e121e0f1-17d3-42c7-98ab-0e739af1be97"
/>


New: 
<img width="1460" height="673" alt="Screenshot 2025-09-07 at 4 30 04 PM"
src="https://github.com/user-attachments/assets/6869a050-15f0-4e17-955d-9e5046fe42e4"
/>
dzorlu pushed a commit to fleet-ai/SkyRL that referenced this pull request Feb 4, 2026
Reverts NovaSky-AI#250

Will run into ` AssertionError: The env var,
__RAY_WORKER_PROCESS_SETUP_HOOK_ENV_VAR, is not permitted because it is
reserved for the internal use.`
dzorlu referenced this pull request in fleet-ai/SkyRL Feb 4, 2026
Re-implement the logging fix of #250 that was reverted in #261

The issue was that using the `worker_process_setup_hook` to set logging
behavior interfered with vLLM using Ray as it's tensor parallel backend
and threw an error. vLLM apparently needs this to be unset.

Moved the logging configuration into RayPPOTrainer `init`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants