Skip to content
New issue

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

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

Already on GitHub? # to your account

A bunch of improvements to logging #3511

Merged
merged 10 commits into from
May 2, 2022
Merged

A bunch of improvements to logging #3511

merged 10 commits into from
May 2, 2022

Conversation

m-col
Copy link
Member

@m-col m-col commented Apr 16, 2022

A few miscellaneous changes to logging:

  • log_utils typing
  • simplifying where logs go: either to file (regular runtime) or stdout (tests/xephyr) but not both
  • remove some redundant code from init_log
  • catch and log exceptions raised within wayland callbacks - currently requires Log exceptions that are raised during callbacks flacjacket/pywayland#38
  • make all log calls the same format, i.e. don't do string interpolation at the call site, let the logger do it, to avoid unnecessary work

@elParaguayo
Copy link
Member

Nice.

If you've changed log output to stdout during tests, does that mean we can revert #3359?

I don't know if that's enough for the caplog fixture but it would be great if your changes were enough.

@m-col
Copy link
Member Author

m-col commented Apr 17, 2022

I don't think it changes the way the logging in the tests works, as that uses a QueueHandler which is independent of the StreamHandler that prints to stdout. They both will continue to receive log messages. I think using the QueueHandler is a good approach. Were you hoping to avoid it if possible?

@elParaguayo
Copy link
Member

Don't need to avoid it. It's more a case of wondering whether your change would make that one unnecessary but I suspect it doesn't and we do need to keep the QueueHandler.

m-col added 9 commits May 2, 2022 14:43
This isn't used anywhere, and is redundant in any case because the log
size is capped to 10MB and so truncation shouldn't ever be needed.
This sets the logger to either log to stdout (tests/xephyr) or a file,
but not both. During regular usage, printing to stdout is pointless.
During tests/dev, logging to file is noise (assuming one is within a
regular qtile session). This sets the default to stdout to simply its
usage (init_log is called much more in tests). This also removes
log_color and ties it to stdout so it does print in colour to stdout,
but not to log.
It's unlikely anybody is calling this themselves, but it is possibly, so
just in case, let's inform users of the change.
This logs exceptions that are risen during Wayland Listener callbacks,
which are currently hidden.

This requires the new release of pywayland.
This makes all logging calls formatted the same way, such that string
interpolation is avoided completely when log messages are below the
current log level.

This also fixes some log calls where we had two adjacent string literals
that were moved onto the same line by black and should instead be merged
into one string literal.
@m-col
Copy link
Member Author

m-col commented May 2, 2022

That pywayland PR was merged so this is ready

@elParaguayo
Copy link
Member

LGTM (other than the test failure...)

@m-col
Copy link
Member Author

m-col commented May 2, 2022

Great thanks. Those were some orphan imports left in some test files so I've removed those and auto-enabled merge.

@m-col m-col enabled auto-merge (rebase) May 2, 2022 18:51
@m-col m-col merged commit 7249cd7 into qtile:master May 2, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants