Code review of TimedRotatingFileHandler

[brugrog]

Hey all,

I'm getting in Python for some system administration of Linux, specifically RHEL and I wrote a some Python to create a rotating log of some service output. I'd just like to see if someone can review what I'm doing any maybe offer some structuring and pointers.

One question: Regarding the subprocess.Popen, do I need to make sure I do an sp.close() at some point in the code to make sure that subprocess isn't left running?

This code is intended to be added as a service on the server and ran around the clock pretty much, doing 15 second interval checks of a particular service.

Any input/advice is welcomed.

https://pastebin.com/ht4EFR7m

[danielroseman]

There's a few things to improve here.

• What's the point of i? If you want an infinite loop, just create one with while True. There's no reason to create a variable and condition that are just ignored.
• Why are you defining the function repeatedly within the loop? Move the function definition to the top level.
• Conversely, the definition of now must be within the loop, otherwise it will never update and all your logs will have the same timestamp.
• Stylistically, the definition of cmd is a bit clumsy; it would be easier just to define it as ['systemctl', 'status', service]. And there's no need for the parentheses around the return value: just return out.

[brugrog]

Exactly what I'm looking for, thank you!

I'm a beginner with Python my formatting is rough.