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

Log message formatting #285

Merged
merged 2 commits into from
Feb 18, 2023
Merged

Log message formatting #285

merged 2 commits into from
Feb 18, 2023

Conversation

johnno1962
Copy link

Hi, was rumaging around trying to find out where my API quota was going and noticed messages were not having values inserted as perhaps you intended.

@jasonacox
Copy link
Owner

Hi @johnno1962 - Thanks for opening this.

Were you getting an error or the wrong data using the current version? There may be something else wrong.

I see the addition of url and headers here. This makes sense and would be a good addition! Thanks for that.

However, moving to the "%" format is not needed for debug(). In fact, pylint will flag this as suboptimal (see W1201 ). Using lazy logging lets the interpreter skip doing the text formatting until it is needed (if debug level was set). It's a small optimization but it is the reason you will see it in the code.

example test

import logging

log = logging.getLogger(__name__)
logging.basicConfig(level=logging.DEBUG)

url = "url"
headers = "headers"
code = 200
text = "text"
token = "token"

log.debug("GET URL=%s HEADERS=%s: response code=%d text=%s token=%s" % (url, headers, code, text, token))

log.debug("GET URL=%s HEADERS=%s: response code=%d text=%s token=%s", url, headers, code, text, token)

@johnno1962
Copy link
Author

johnno1962 commented Feb 18, 2023

Ha, shows how much Python I know. So, the log function is performing the % expansion on demand which is as it should be. You can disregard this PR or, I've pushed another commit to include only the URL=%s HEADERS=%s change. Thanks again for this invaluable resource. I've been able to talk to the cloud from Swift but exhausted my quota for this month :D

@jasonacox jasonacox merged commit 37c1351 into jasonacox:master Feb 18, 2023
@jasonacox
Copy link
Owner

Thanks @johnno1962 ! It's a good addition.

Thanks to this great community, and contributors like you, the project keeps getting better. ;)

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.

2 participants