msg112800 - (view) |
Author: Nick Craig-Wood (ncw) * |
Date: 2010-08-04 13:18 |
sqlite3.Warning isnt a subclass of exceptions.Warning
This causes this problem when trying to filter warnings
>>> import sqlite3 as DB
>>> from warnings import filterwarnings
>>> filterwarnings("always", category=DB.Warning)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python2.6/warnings.py", line 56, in filterwarnings
assert issubclass(category, Warning), "category must be a Warning subclass"
AssertionError: category must be a Warning subclass
>>>
Other databases do this correctly, eg
>>> import MySQLdb as DB
>>> from warnings import filterwarnings
>>> filterwarnings("always", category=DB.Warning)
>>>
|
msg112801 - (view) |
Author: Nick Craig-Wood (ncw) * |
Date: 2010-08-04 13:19 |
I've attached a patch to fix the issue along with a revised test.
|
msg112803 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-08-04 13:31 |
I agree this is a bug, and the patch looks good. This breaks compatibility though, so I’m not sure it can make it before 3.2 or 3.3.
|
msg112804 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-08-04 13:34 |
I don't really think it breaks compatibility.
The patch is bad, though. The "exceptions" module doesn't exist anymore in 3.x, the Warning class should be referenced directly instead.
|
msg112805 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-08-04 13:36 |
I mean that sqlite3.Warning used to be a subclass of StandardError, and there may be code relying on that.
|
msg112806 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-08-04 13:38 |
Ah, I see. Well I don't think we can change inheritance in bugfix branches anyway.
In 3.x, though, StandardError has disappeared and sqlite3.Warning inherits directly from Exception:
>>> import sqlite3
>>> sqlite3.Warning.__bases__
(<class 'Exception'>,)
|
msg112808 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-08-04 13:40 |
Warning is a subclass of Exception, so perfect.
Please add 2.7 and 3.1 if you agree this bugfix should go there too.
|
msg112817 - (view) |
Author: Nick Craig-Wood (ncw) * |
Date: 2010-08-04 15:00 |
I re-worked the patch for python 3.x (py3k branch) - the other was for 2.x (trunk)
Basically the same patch and fixes the issue according to my testing
|
msg112819 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-08-04 15:08 |
Nitpick: s/built in/built-in/ (don’t update your patch for that, the person who will commit can do it)
|
msg112820 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-08-04 15:20 |
On the other hand, the sqlite3 source code uses PyErr_Set*() rather than PyErr_Warn*(), so there's any point in trying to filter the warnings out (it won't work).
Looking at the kind of problems that it is meant to reflect, "Warning" is actually a misnomer, since they are genuine errors (PYSQLITE_TOO_MUCH_SQL, PYSQLITE_SQL_WRONG_TYPE) which you cannot simply ignore.
|
msg112963 - (view) |
Author: Nick Craig-Wood (ncw) * |
Date: 2010-08-05 09:38 |
I think the fact that sqlite may not be using the warnings properly is independent of this problem. Warnings should be filterable, but if sqlite isn't notifying them properly - that would be a different bug.
BTW I came across this problem when trying to swap out MySQLdb with sqlite3. The existing code was filtering warnings, that didn't work with sqlite3, hence this bug report!
|
msg112964 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-08-05 09:40 |
I agree with Antoine. You say you want to filter warnings, but you can’t since they are not warnings.
|
msg112966 - (view) |
Author: Gerhard Häring (ghaering) * |
Date: 2010-08-05 09:57 |
PEP 0249 says that the module's Warning class must be a subclass of StandardError. So I reject your proposed change.
There are only two cases where pysqlite raises Warning, and these could be changed to ProgrammerError anyway.
|
msg112977 - (view) |
Author: Nick Craig-Wood (ncw) * |
Date: 2010-08-05 13:18 |
Reading PEP 0249 I can see Gerhard is correct, this patch would violate the PEP.
I think that the PEP is slightly flawed in that users are encouraged to raise exceptions called "Warning". IMHO a Warning is never an exceptional condition and should be notified by the warnings framework.
This obviously confused the authors of MySQLdb, who do indeed warn() their Warning classes rather than raise() them, and it is very useful to be able to filter them.
To obey the letter of the PEP the authors of the MySQLdb interface multiply inherit their Warning class from exceptions.StandardError and exceptions.Warning.
I could make a patch for sqlite3 to do this if anyone thinks it would be useful.
|
msg148278 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-11-24 16:38 |
> Reading PEP 0249 I can see Gerhard is correct, this patch would violate the PEP.
Me too.
> I think that the PEP is slightly flawed in that users are encouraged to raise exceptions
> called "Warning". IMHO a Warning is never an exceptional condition and should be notified
> by the warnings framework.
That’s probably a bad word choice in the PEP.
> This obviously confused the authors of MySQLdb, who do indeed warn() their Warning classes
> rather than raise() them, and it is very useful to be able to filter them.
> To obey the letter of the PEP the authors of the MySQLdb interface multiply inherit their
> Warning class from exceptions.StandardError and exceptions.Warning.
I think they are mistaken, given that the DB API warnings are actually errors.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:04 | admin | set | github: 53719 |
2011-11-24 16:38:14 | eric.araujo | set | messages:
+ msg148278 stage: patch review -> resolved |
2010-08-05 13:18:43 | ncw | set | messages:
+ msg112977 |
2010-08-05 09:57:04 | ghaering | set | status: open -> closed assignee: ghaering resolution: rejected messages:
+ msg112966
|
2010-08-05 09:40:35 | eric.araujo | set | messages:
+ msg112964 |
2010-08-05 09:38:39 | ncw | set | messages:
+ msg112963 |
2010-08-04 15:20:53 | pitrou | set | messages:
+ msg112820 |
2010-08-04 15:08:20 | eric.araujo | set | messages:
+ msg112819 |
2010-08-04 15:00:21 | ncw | set | files:
+ sqlite3-warning-fix-py3k.patch
messages:
+ msg112817 |
2010-08-04 13:40:09 | eric.araujo | set | messages:
+ msg112808 |
2010-08-04 13:38:23 | pitrou | set | messages:
+ msg112806 |
2010-08-04 13:36:17 | eric.araujo | set | messages:
+ msg112805 |
2010-08-04 13:34:48 | pitrou | set | nosy:
+ ghaering, pitrou
messages:
+ msg112804 stage: commit review -> patch review |
2010-08-04 13:31:48 | eric.araujo | set | versions:
+ Python 3.2, - Python 2.7 nosy:
+ eric.araujo
messages:
+ msg112803
stage: commit review |
2010-08-04 13:19:44 | ncw | set | messages:
+ msg112801 |
2010-08-04 13:18:10 | ncw | create | |