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

Provided better example for logging cookbook (GH-101164) #101164

Merged
merged 6 commits into from
Jan 20, 2023
Merged

Provided better example for logging cookbook (GH-101164) #101164

merged 6 commits into from
Jan 20, 2023

Conversation

galqiwi
Copy link
Contributor

@galqiwi galqiwi commented Jan 19, 2023

This change is trivial, so, according to guidelines, i haven't created an issue.

I just made a small fix to the code from logging cookbook. The rotator function should compress file. And, to compress file, you need to use gzib library, not zlib.

To save you some time, this fact is clearly stated in zlib docs

For reading and writing .gz files see the gzip module.

@galqiwi galqiwi requested a review from vsajip as a code owner January 19, 2023 20:28
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Jan 19, 2023
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Jan 19, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Member

@vsajip vsajip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're doing this, we might as well change this snippet into a runnable script:

import gzip
import logging
import logging.handlers
import os
import shutil

def namer(name):
    return name + '.gz'

def rotator(source, dest):
    with open(source, 'rb') as f_in:
        with gzip.open(dest, 'wb') as f_out:
            shutil.copyfileobj(f_in, f_out)
    os.remove(source)

rh = logging.handlers.RotatingFileHandler('rotated.log', maxBytes=1024, backupCount=5)
rh.rotator = rotator
rh.namer = namer
root = logging.getLogger()
root.setLevel(logging.INFO)
root.addHandler(rh)
f = logging.Formatter('%(asctime)s %(message)s')
rh.setFormatter(f)
for i in range(1000):
    root.info(f'Message no. {i + 1}')

and show that it seems to work:

$ gzip -dc rotated.log.1.gz | head -n 5
2023-01-19 21:55:12,677 Message no. 955
2023-01-19 21:55:12,678 Message no. 956
2023-01-19 21:55:12,678 Message no. 957
2023-01-19 21:55:12,678 Message no. 958
2023-01-19 21:55:12,709 Message no. 959

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@galqiwi
Copy link
Contributor Author

galqiwi commented Jan 19, 2023

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@vsajip: please review the changes made to this pull request.

@vsajip vsajip changed the title Provided better example for logging cookbook Provided better example for logging cookbook (GH-101164) Jan 20, 2023
@vsajip vsajip merged commit 8cdbc46 into python:main Jan 20, 2023
@vsajip vsajip added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Jan 20, 2023
@miss-islington
Copy link
Contributor

Thanks @galqiwi for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @galqiwi for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-101183 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 20, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 20, 2023
(cherry picked from commit 8cdbc46)

Co-authored-by: Vladimir Malinovskii <galqiwi@galqiwi.ru>
Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
@bedevere-bot
Copy link

GH-101184 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 20, 2023
(cherry picked from commit 8cdbc46)

Co-authored-by: Vladimir Malinovskii <galqiwi@galqiwi.ru>
Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jan 20, 2023
@vsajip
Copy link
Member

vsajip commented Jan 20, 2023

@galqiwi : Thanks for this patch, now merged, but you signed the CLA using your @yandex-team.ru. However, this information was lost when the PR was squashed and merged, and the backport PRs can't be merged because they only have your GitHub profile email address galqiwi@galqiwi.ru, which is not recorded as having signed the CLA. Can you please sign the CLA again, but using your @galqiwi.ru email? Thanks!

vsajip added a commit that referenced this pull request Jan 20, 2023
…01183)

Co-authored-by: Vladimir Malinovskii <galqiwi@galqiwi.ru>
Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
vsajip added a commit that referenced this pull request Jan 20, 2023
…01184)

Co-authored-by: Vladimir Malinovskii <galqiwi@galqiwi.ru>
Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants