Skip to content

feat: add ability to log to a file#885

Open
carlosm27 wants to merge 16 commits into
sparckles:mainfrom
carlosm27:log_file
Open

feat: add ability to log to a file#885
carlosm27 wants to merge 16 commits into
sparckles:mainfrom
carlosm27:log_file

Conversation

@carlosm27

Copy link
Copy Markdown
Contributor

Description

This PR fixes #841

This PR adds the ability to create a log file.
This is an example of how to do it:

Screenshot 2024-07-07 230957

Screenshot 2024-07-07 230937

@vercel

vercel Bot commented Jul 8, 2024

Copy link
Copy Markdown

@carlosm27 is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

@carlosm27

Copy link
Copy Markdown
Contributor Author

Should we add the ability to add more configurations, like timestamps, levels, etc?

@codspeed-hq

codspeed-hq Bot commented Jul 8, 2024

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #885 will not alter performance

Comparing carlosm27:log_file (6a5c4d5) with main (51bf8a3)

Summary

✅ 146 untouched benchmarks

@carlosm27 carlosm27 changed the title feat : add ability to log to a file feat: add ability to log to a file Jul 8, 2024
@sansyrox

sansyrox commented Jul 9, 2024

Copy link
Copy Markdown
Member

Hey @carlosm27 👋

How would you use this new interface?

@carlosm27

Copy link
Copy Markdown
Contributor Author

Hey @carlosm27 👋

How would you use this new interface?

Hey @sansyrox. I would use it to create log files with timestamps, log level and with and add rotation policy.

What features are more appropiate for this interface?

@vercel

vercel Bot commented Nov 18, 2024

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robyn ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 18, 2024 0:22am

@recurseml

recurseml Bot commented May 27, 2025

Copy link
Copy Markdown

✨ No issues found! Your code is sparkling clean! ✨

@sansyrox sansyrox left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tackling this, @carlosm27 — file logging (#841) is genuinely useful and the FileHandler approach is the right idea. A couple of things before we can merge: the PR accidentally commits a compiled Windows binary (robyn/robyn.cp311-win_amd64.pyd) which needs to be removed, and the changes to integration_tests/index.py are demo snippets rather than an actual test. One functional concern: our log messages are wrapped with ANSI color codes via _format_msg, so they'd get written into the file as raw escape sequences — could you strip colors for the file handler? Finally, a quick rename to something like log_to_file would read more naturally. Happy to help get this over the line once those are addressed!

@sansyrox sansyrox added reviewed Reviewed by a maintainer changes-requested Mergeable but needs changes before merge labels Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes-requested Mergeable but needs changes before merge reviewed Reviewed by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an ability to log to a file

3 participants