Skip to content
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.

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

Revert "Pass params into logger.{info,debug}" #923

Merged
merged 1 commit into from Feb 6, 2023

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Feb 6, 2023

r? @pakrym-stripe

Reverts #913

fixes #922

I think our strategy here is wrong.

$ python -c "import logging;help(logging.Logger.info)" | cat
Help on function info in module logging:

info(self, msg, *args, **kwargs)
    Log 'msg % args' with severity 'INFO'.

The default behavior is to log msg % args but we definitely don't actually want to do this. Our intention was to allow the user to access this via a custom handler, but if the default behavior is for the logging handler to calculate msg % args this is not the right way to do this.

@richardm-stripe richardm-stripe enabled auto-merge (squash) February 6, 2023 18:01
@richardm-stripe richardm-stripe merged commit 7b591fd into master Feb 6, 2023
@richardm-stripe richardm-stripe deleted the revert-913-richardm-extensible-logger branch February 6, 2023 18:13
richardm-stripe added a commit that referenced this pull request Feb 24, 2023
* Pass params into logger.{info,debug} (#913)

* Pass params into logger.{info,debug}

* API Updates (#920)

* Codegen for openapi v223

* Bump version to 5.1.0

* Revert "Pass params into logger.{info,debug} (#913)" (#923)

This reverts commit c45cc1d.

* Bump version to 5.1.1

* Codegen for openapi v225

* Bump version to 5.2.0

* Set version to 5.2.0 to simplify merge

* Reset version to 5.2.0b1

* Codegen for openapi v232

* Format

---------

Co-authored-by: Annie Li <anniel@stripe.com>
Co-authored-by: anniel-stripe <97691964+anniel-stripe@users.noreply.github.com>
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.

Recent logging change breaks under tests
1 participant