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

Peewee now logs Errors that were previously caught #254

Closed
chrismatta opened this issue Nov 20, 2013 · 13 comments
Closed

Peewee now logs Errors that were previously caught #254

chrismatta opened this issue Nov 20, 2013 · 13 comments

Comments

@chrismatta
Copy link

I think this has to do with the 2.1.5 sql_error_handler. Previously to know if I should update a record I would try a save and catch a MySQL IntegrityError, then I would update any fields in the duplicate record like this:

class BaseModel(Model):

    class Meta:
        database = mysql_db


class User(BaseModel):
    firstname = CharField(index=True)
    lastname = CharField(index=True)
    zipcode = IntegerField()

    class Meta:
        db_table = 'user'
        indexes = (
            (('firstname', 'lastname'), True),
        )

def main():
    User.create_table(True)

    users = [('Joe', 'Dirt', 11111),
             ('Ron', 'Burgundy', 22222),
             ('Joe', 'Dirt', 33333)]

    for user in users:
        firstname, lastname, zipcode = user

        user_inst = User(firstname=firstname,
            lastname=lastname, zipcode=zipcode)

        try:
            user_inst.save()
        except IntegrityError:
            User.update(
                zipcode=zipcode
            ).where(
                User.firstname == firstname,
                User.lastname == lastname
            ).execute()

And this still works but now the peewee module is logging SQL errors:

ERROR:peewee:Error executing query INSERT INTO `user` (`lastname`, `firstname`, `zipcode`) VALUES (%s, %s, %s) ([u'Dirt', u'Joe', 33333])

I'd rather it didn't log those errors at all, but the only way to suppress them that I can see is to disable peewee logging at the logging.ERROR level and I don't think that's wise.

I like this technique better than doing a try .get() for each record and inserting on a DoesNotExist exception catch, but I don't like all the errors, any way around it?

@coleifer
Copy link
Owner

The only things peewee logs are:

  • debug: log every query
  • error: log queries that result in errors

So if you do not want these errors, you can just add a null handler to the logger:

import logging
logger = logging.getLogger('peewee')
logger.addHandler(logging.NullHandler())  # suppress logging

@chrismatta
Copy link
Author

Thanks, do you have any insight in an alternative way to update a record on duplicate keys?

@coleifer
Copy link
Owner

I think the way you're doing it makes sense. Do I need to address anything else regarding the behavior of the logging?

@chrismatta
Copy link
Author

Adding a NullHandler() didn't seem to work, it's still logging every duplicate record insert error, here's my logging section definition in the module that calls the db stuff:

logging.basicConfig()
peewee_logger = logging.getLogger('peewee')
peewee_logger.addHandler(logging.NullHandler())
logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)

@chrismatta
Copy link
Author

I use this technique a lot and know that the save() may or may not succeed, that's the whole point of catching it, I don't need to know about the error. I would however like to know about OTHER SQL errors that I may or may not be catching. This seems like I may need to change my technique for updating existing records so as to not run afoul of all these errors.

@chrismatta
Copy link
Author

Turns out I use the other technique (check for existence catch DoesNotExist then create) a lot more, so I just switched over to that. This is fine now, sorry for the trouble.

@coleifer
Copy link
Owner

Adding a NullHandler() didn't seem to work, it's still logging every duplicate record insert error, here's my logging section definition in the module that calls the db stuff:

You might try then peewee_logger.handlers = []. The logging docs are pretty helpful.

Turns out I use the other technique (check for existence catch DoesNotExist then create) a lot more, so I just switched over to that. This is fine now, sorry for the trouble.

Sounds good!

@xealot
Copy link

xealot commented Mar 25, 2014

Attempting to insert a record and relying on the referential integrity of the database to tell you if a row is present is a very common use case. Having peewee spam the logger with this information (especially when it is caught) is silly.

Imagine you are inserting 10 million rows into a database. About 10% of your records have duplicate keys in one table or another. Checking before every insert would double the amount of DB queries required to complete the task.

Turning off logging for peewee is a nuclear solution to this issue since it will disable your ability to see any other useful messages. Also setting the level above error would essentially only permit critical messages to pass through.

I really appreciate the detail which peewee uses for the logger and how easy it is to see what's going on. However sending an exception to the logger inside of the library doesn't allow your library user to properly handle the exception. Even though its caught, your logs would still be full of exceptions.

@antback
Copy link

antback commented Mar 26, 2014

I totally agree with the last comment. I'm fighting with the same issue. I don't want to lose all the error traces just to ignore something that is not an error.

I don't want to miss the oportunity to tell you what a great framework. Good job. Thanks a lot.

@coleifer coleifer reopened this Mar 26, 2014
@coleifer
Copy link
Owner

I really appreciate the detail which peewee uses for the logger and how easy it is to see what's going on. However sending an exception to the logger inside of the library doesn't allow your library user to properly handle the exception. Even though its caught, your logs would still be full of exceptions.

What would you like to see instead?

@xealot
Copy link

xealot commented Mar 26, 2014

I would prefer that peewee did not log the exception to the logger but instead allowed the project that includes peewee to handle and/or log the error.

From the perspective of a library which is solely included in another project, peewee should not impair the ability of the project owner to appropriately catch and handle errors. I would expect the library to throw the exception and be done.

@coleifer
Copy link
Owner

coleifer commented Apr 1, 2014

I would prefer that peewee did not log the exception to the logger but instead allowed the project that includes peewee to handle and/or log the error.

That makes sense to me.

@xealot
Copy link

xealot commented Apr 1, 2014

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants