gilesknap / gphotos-sync

Google Photos and Albums backup with Google Photos Library API
Apache License 2.0
1.97k stars 163 forks source link

reduce log level for informative things from WARNING to INFO for less pollution of logs / cron #362

Closed nohn closed 2 years ago

gilesknap commented 2 years ago

Yep, not sure why I made this warnings in the first place.

codecov[bot] commented 2 years ago

Codecov Report

Merging #362 (318b159) into main (ec1840a) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #362   +/-   ##
=======================================
  Coverage   89.21%   89.21%           
=======================================
  Files          25       25           
  Lines        1957     1957           
=======================================
  Hits         1746     1746           
  Misses        211      211           
Impacted Files Coverage Δ
src/gphotos_sync/GooglePhotosDownload.py 81.90% <100.00%> (ø)
src/gphotos_sync/GooglePhotosIndex.py 94.89% <100.00%> (ø)
src/gphotos_sync/Main.py 88.71% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ec1840a...318b159. Read the comment docs.

gilesknap commented 2 years ago

@nohn I think we should make the default log level INFO. Otherwise many users will be surprised by lack of output.

gilesknap commented 2 years ago

@nohn OK now I remember why:

with log level INFO I get

05-28 18:10:52 INFO     downloading 90 photos/2022/04/IMG_1839.HEIC 
05-28 18:10:52 INFO     downloading 91 photos/2022/04/IMG_1838.HEIC 
05-28 18:10:52 INFO     downloading 92 photos/2022/04/IMG_1837.HEIC 
05-28 18:10:52 INFO     downloading 93 photos/2022/04/IMG_1836.HEIC 
05-28 18:10:52 INFO     downloading 94 photos/2022/04/IMG_1835.HEIC 
05-28 18:10:53 INFO     downloading 95 photos/2022/04/IMG_1834.HEIC 
05-28 18:10:53 INFO     downloading 96 photos/2022/04/IMG_1833.HEIC 
05-28 18:10:54 INFO     downloading 97 photos/2022/04/IMG_1832.HEIC 
05-28 18:10:54 INFO     downloading 98 photos/2022/04/IMG_1831.HEIC 
05-28 18:10:55 INFO     downloading 99 photos/2022/04/IMG_1830.HEIC 
05-28 18:10:55 INFO     downloading 100 photos/2022/04/IMG_1829.HEIC 
05-28 18:10:55 INFO     downloading 101 photos/2022/04/IMG_1828.HEIC 
05-28 18:10:55 INFO     downloading 102 photos/2022/04/IMG_1827.HEIC 

etc.

So what I needed was an INFO and a VERBOSE INFO log level. You can add custom log levels to python logging. Maybe this is the way to go?

gilesknap commented 2 years ago

Just remembered that gphotos-sync already has the custom logging level TRACE so should be easy to:

nohn commented 2 years ago

Well. There are current WARNINGS that I would like to see. Like photos NOT being downloaded.

gilesknap commented 2 years ago

Well. There are current WARNINGS that I would like to see. Like photos NOT being downloaded.

Yes I'm suggesting that what you left as warning is still left as warning. But what was already INFO should be changed to VERBOSE.

nohn commented 2 years ago

Is what I recently pushed what you are suggesting?

gilesknap commented 2 years ago

I'm afraid this does not run because you need to add the custom log level verbose in Logging.py e.g. trace is added here https://github.com/gilesknap/gphotos-sync/blob/ec1840a0009373fd309508f59fe0e6a793d81d32/src/gphotos_sync/Logging.py#L26-L28

Also I think you changed all infos to verbose. That's not quite right, lets see if I can explain better.

Take the original code before your PR and :

That's it. Do you think that is a sensible change that meets your requirement (for your requirement you would need to use --log-level WARNING -- for people that are happy with the way things were it will output the same information except that the 'WARNING' tags would change to 'INFO')

gilesknap commented 2 years ago

I've granted rights to run the CI so that you can see if the tests still work when you push changes to the PR.