feat: add back the capturing of logs from logging#418
Open
bepri wants to merge 1 commit intofeature/oxidationfrom
Open
feat: add back the capturing of logs from logging#418bepri wants to merge 1 commit intofeature/oxidationfrom
logging#418bepri wants to merge 1 commit intofeature/oxidationfrom
Conversation
bepri
commented
Feb 13, 2026
| greeting: String, | ||
|
|
||
| /// A handle on log handling. | ||
| _log_handle: Option<Py<PyAny>>, |
Member
Author
There was a problem hiding this comment.
I keep this so Python doesn't deallocate it.
bepri
commented
Feb 13, 2026
| py: Python<'_>, | ||
| verbosity: Verbosity, | ||
| ) -> PyResult<Option<Py<PyAny>>> { | ||
| let log_handler = match LogListener::new(py, verbosity)? { |
Member
Author
There was a problem hiding this comment.
If this returns None, it's because we decided that log capture wasn't necessary for this run's verbosity level.
bepri
commented
Feb 13, 2026
| #[pymethods] | ||
| impl LogListener { | ||
| fn emit(&mut self, py: Python<'_>, record: &Bound<'_, PyAny>) -> PyResult<()> { | ||
| let levelno = record.getattr(intern!(py, "levelno"))?; |
Member
Author
There was a problem hiding this comment.
This intern! macro is just a cheap optimization trick PyO3 provides. Instead of having to coerce the Rust "levelno" string into Python for every logging event, intern! keeps a copy of this string on Python's heap after the first call.
bepri
commented
Feb 13, 2026
| pub fn new(py: Python<'_>, verbosity: Verbosity) -> PyResult<Option<Self>> { | ||
| let res = match verbosity { | ||
| Verbosity::Quiet => None, | ||
| Verbosity::Brief => None, |
Member
Author
There was a problem hiding this comment.
This is separated out from Quiet because Brief mode can require log handling if streaming_brief is enabled, which will be the next PR.
3 tasks
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.
make lint && make test?This PR brings back Craft CLI's integration with the
logginglibrary. To do this, a shim class is defined incraft_cli/_logs.pyin order to inherit fromlogging.Handler. This class wraps aLogListenerclass defined in Rust that actually parses logging events and sends them to the printer.Here's a quick script for testing the feature:
I discovered a bug while testing where
BRIEFverbosity will often clear one too many lines of your terminal, but it isn't related to these changes.