FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Do not setup log handlers to write to stderr by draftcode · Pull Request #758 · testcontainers/testcontainers-python · GitHub

Do not setup log handlers to write to stderr - #758

Closed
draftcode wants to merge 1 commit into
testcontainers:mainfrom
draftcode:log_handler_setup
Closed

Do not setup log handlers to write to stderr#758
draftcode wants to merge 1 commit into
testcontainers:mainfrom
draftcode:log_handler_setup

Conversation

Copy link
Copy Markdown

By setting up a handler in the library, the library users do not have a good control on how/where to write the logs. Do not set a handler from the library.

By setting up a handler in the library, the library users do not have a
good control on how/where to write the logs. Do not set a handler from
the library.

alexanderankin commented Jan 15, 2025
edited
Loading

Copy link
Copy Markdown
Member

CarliJoy commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

@alexanderankin what do you mean with a logging overhaul?

Logging is always quite language specific as it requires global objects.

I am with draftcode that a libary should not setup a logger, it only should log.

I would even go that far, that the library should not even define the log level. It up to the user of the lib to define it.
But I see it is convenient to this. But at least do it in away, that is done exactly once during import for all modules.

I.e.

import logging
from typing import Final

LOGGER: Final = logging.getLogger("testcontainers")
LOGGER.setLevel(logging.INFO)

This would set the log level to all testcontainers logger (i.e. also testcontainers.core.utils).

Besides this, there is nothing more to do.

Why this is important

I am using testcontainers in pytest plugin, as I need containers to be started before imports are done (legacy code, that setups databases during import...).
And there is a long standing issues with pytest that doesn't handle it well when prints are happening in certain conditions.

So being able to simply disable/change the handler all together would be nice.

Copy link
Copy Markdown
Contributor

Agree 💯 here. I'd go so far as to suggest setup_logger be removed from the library. All callers should be replaced with simply logging.getLogger

Copy link
Copy Markdown
Member

closed in favor of #836

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL