meraki / dashboard-api-python

Official Dashboard API library (SDK) for Python
MIT License
289 stars 151 forks source link

Fix logging #168

Closed mm-irvingleonard closed 2 years ago

mm-irvingleonard commented 2 years ago

Is there a reason for your approach to logging? It's extremely weird to see a library messing with the logging configuration, and mostly useless.

What's wrong with the simple and basic approach? Just define a logger (or several, if that's what you want) and log away. No need to mess with log formats, or levels, or output, or anything like that. You're assuming your user's intentions and that's extremely complicated and usually impossible to get right; is best to leave that bit to the actual user to figure out.

BTW, I would say that a library should generate debug logs (for debug purposes, obviously) and warning/errors/exceptions when things go wrong. There's almost no reason for a library to generate info logs, but that might be just me.

Right now the best I can do is "suppress_logging" since none of your "configurations" fit my needs (I already have code to handle logging config, which you basically ignore)

TKIPisalegacycipher commented 2 years ago

Hello @mm-irvingleonard,

Thank you for the candid feedback. This is an open source library and we welcome suggestions especially in the form of pull requests. If you would like to submit one that addresses these issues then I think we could have a serious conversation about changing the current behavior. I wouldn't say anyone is particularly married to the current implementation, but finding cycles to make changes is usually harder. I don't see any evidence that using external loggers was ever in scope for this library.

Please do reach out if you have any other concerns.