Skip to content

add option for log_file to setup_logging #723

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.

Sign up for GitHub

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

Open
rasdani wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add option for log_file to setup_logging #723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee36555
add option for `log_file` to `setup_logging`
rasdani Jan 13, 2026
8805864
update docs
rasdani Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ vf.print_prompt_completions_sample(outputs: GenerateOutputs, n: int = 3)
Pretty-print sample rollouts.

```python
vf.setup_logging(level: str = "INFO")
vf.setup_logging(level: str = "INFO", log_file: str | None = None)
```

Configure verifiers logging. Set `VF_LOG_LEVEL` env var to change default.
Configure verifiers logging. Set `VF_LOG_LEVEL` env var to change default. If `log_file` is provided, logs to that file instead of stderr.
9 changes: 7 additions & 2 deletions verifiers/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def setup_logging(
level: str = "INFO",
log_format: Optional[str] = None,
date_format: Optional[str] = None,
log_file: Optional[str] = None,
) -> None:
"""
Setup basic logging configuration for the verifiers package.
Expand All @@ -51,14 +52,18 @@ def setup_logging(
level: The logging level to use. Defaults to "INFO".
log_format: Custom log format string. If None, uses default format.
date_format: Custom date format string. If None, uses default format.
log_file: Path to log file. If None, logs to stderr.
"""
if log_format is None:
log_format = "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
if date_format is None:
date_format = "%Y-%m-%d %H:%M:%S"

# Create a StreamHandler that writes to stderr
handler = logging.StreamHandler(sys.stderr)
# Create handler - file or stderr
Copy link
Member

@mikasenghaas mikasenghaas Jan 13, 2026

Choose a reason for hiding this comment

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

should we not rather support console + file? ie. file logging is an add-on, not a replacement

Copy link
Contributor Author

@rasdani rasdani Jan 13, 2026

Choose a reason for hiding this comment

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

then it defies the purpose of keeping orchestrator logs clean via files.
i would then just handle streams in prime-rl natively.
https://github.com/PrimeIntellect-ai/prime-rl/pull/1561/changes

Copy link
Contributor Author

@rasdani rasdani Jan 13, 2026

Choose a reason for hiding this comment

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

i can also do

  vf.setup_logging(level=vf_log_level.upper())                                                                                                                                                                         
  # Redirect verifiers to file instead of inherited stderr                                                                                                                                                             
  vf_logger = logging.getLogger("verifiers")                                                                                                                                                                           
  vf_logger.handlers.clear()                                                                                                                                                                                           
  vf_logger.addHandler(logging.FileHandler(log_file))

in prime-rl. thought it was cleaner to have it live in vf.

if log_file is not None:
handler = logging.FileHandler(log_file)
else:
handler = logging.StreamHandler(sys.stderr)
Copy link

@cursor cursor bot Jan 13, 2026

Choose a reason for hiding this comment

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

FileHandler not closed before clearing handlers

Low Severity

When setup_logging is called multiple times with log_file specified, logger.handlers.clear() removes the old handler from the list without calling handler.close() first. With the previous StreamHandler(sys.stderr) this was benign since stderr is always open, but with the new FileHandler this leaks file descriptors. Each subsequent call leaves the previous log file handle open.

Additional Locations (1)

Fix in Cursor Fix in Web

handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=date_format))

# Get the root logger for the verifiers package
Expand Down
Loading