Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging update. Fixed regression and update to Docker log handling. #1066

Merged
merged 4 commits into from
Jan 9, 2025

Conversation

kga245
Copy link
Contributor

@kga245 kga245 commented Jan 7, 2025

  1. Event logging fixed to populate JSON log (appears to have been a regression recently re-introduced.
  2. Added tests to make sure event logging creates a requesite file.
  3. Changed way docker logs font issues from WeasyPrint to be less verbose.

kga245 added 3 commits January 6, 2025 21:24
Problem:
- Main application wasn't properly logging research events to output files
- Log files were being created but weren't capturing events

Solution:
- Fixed researcher initialization chain in main application
- Modified run_agent() to properly use CustomLogsHandler
- Events now correctly captured in output files

Testing:
- Verified through uvicorn main:app --reload
- Events properly logged to outputs/task_*.json
- Add test to verify researcher event logging
- Test ensures events are properly captured in output files
- Validates log file structure and content
- Confirms proper initialization of CustomLogsHandler
- Added explicit logging level configuration in docker-compose.yml
- Suppressed verbose fontTools logging messages in main.py
@kga245 kga245 changed the title Reduce docker logging Logging update. Fixed regression and update to Docker log handling. Jan 7, 2025
@kga245
Copy link
Contributor Author

kga245 commented Jan 8, 2025

Noticed that logs didn't sync to OS when using docker. Wanted to expose those and all outputs when running docker.

@assafelovic
Copy link
Owner

Hey @kga245 can i merge this after tests?

@kga245
Copy link
Contributor Author

kga245 commented Jan 8, 2025

Yep, I expected that!

@kga245
Copy link
Contributor Author

kga245 commented Jan 8, 2025

FWIW I am bringing in a full-time tester. We're going to put this on the rails!

@kga245
Copy link
Contributor Author

kga245 commented Jan 8, 2025

FYI, running as uvicorn/fastapi on a heroku dyno produces some very verbose logs which is actually desired behavior. I didn't anticipate that but it's a happy accident.

@assafelovic
Copy link
Owner

So excited to have you onboard! @kga245 🚀

@Mizokuiam
Copy link

Thank you for raising this issue! I'll look into it and try to help if I can.

@assafelovic assafelovic merged commit 10bdeec into assafelovic:master Jan 9, 2025
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.

3 participants