piskvorky / sqlitedict

Persistent dict, backed by sqlite3 and pickle, multithread-safe.
Apache License 2.0
1.16k stars 130 forks source link

Change logging from info to debug #163

Closed nvllsvm closed 1 year ago

nvllsvm commented 1 year ago

What Change the the database-open log from info-level to debug-level

Why It's an implementation detail that one does not need to care about while using this library.

mpenkov commented 1 year ago

Dunno, I think your changes are OK, but I also think it's fine the way it is, because:

  1. This log statement gets executed rarely
  2. Whether or not people care about this statement is really up to them
  3. If they really don't care, then they can set the logging level to something else when configuring the logging library

@piskvorky thoughts?

piskvorky commented 1 year ago

I'd keep it as-is, mostly on account of @mpenkov 's point 3). That's the reason why we use logging, so that people can configure the threshold the way they like.

While it's true there is some subjective arbitrariness in what is considered DEBUG (for developers) vs INFO (generally useful info but nothing vital), IMO "opening a database" falls squarely into INFO.

@nvllsvm why does it bother you?

nvllsvm commented 1 year ago

Let me start by saying that I understand I'm being fairly pedantic about not liking the default of an info-level log entry. Thank you for discussing with me.

why does it bother you?

It's unexpected to have an otherwise unopinionated library output info-level statements by default. Libraries like the stdlib's sqlite3 and the popular pycopg do not output anything info-level when connecting to their respective databases. Tangentially, neither httpx not requests output info-level when making an HTTP request. These libraries delegate that responsibility to the developer using them.

For example, the only log entry output by this dict is from sqlitedict

import logging
import sqlite3

import psycopg
import requests
import sqlitedict

logging.basicConfig(level=logging.INFO)

sqlite3.connect('test.sqlite')
psycopg.connect('postgresql://somedb')
requests.get('https://github.com')
sqlitedict.SqliteDict('test.sqlitedict')
INFO:sqlitedict:opening Sqlite table 'unnamed' in 'test.sqlitedict'

That log entry is unnecessary noise in log output - especially when the output is shared with someone unfamiliar with sqlitedict or when multiple files are being used.

I know I can manually change the level of sqlitedict's logger, but then that becomes noise in the code - which results in a very slight increase in complexity for someone not familiar with sqlitedict (my team).

I also use this library in multiple, self-contained scripts so having to disable the library's opinion in each one is just one more thing I have to do.

mpenkov commented 1 year ago

That log entry is unnecessary noise in log output - especially when the output is shared with someone unfamiliar with sqlitedict or when multiple files are being used.

OK, that's a reasonable point.

mpenkov commented 1 year ago

Merged, thank you for your effort and explanation.