sqlite backend EagerCursor.execute does not return self

[vince@…]

In trac.db.sqlite_backend.py, EagerCursor inherits from PyFormatCursor, but does not return a value like PyFormatCursor does.

This seems to cause several problems with my Trac project due to inconsistent behavior. Namely, several places that use the chaining to call fetchone() or fetchall().

        def execute(self, *args):
            PyFormatCursor.execute(self, *args)
            self.rows = PyFormatCursor.fetchall(self)
            self.pos = 0

There appears to be a simple solution:

        def execute(self, *args):
            PyFormatCursor.execute(self, *args)
            self.rows = PyFormatCursor.fetchall(self)
            self.pos = 0
            return self

[Remy Blank]

Slightly different patch applied to 0.11-stable (as that's where EagerCursor was introduced) in [8916]. Thanks!

[Christian Boos]

Too quick Remy, I would have closed that as wontfix, as we're following the dbapi2 (​pep:0249) in which the result of Cursor.execute is explicitly said to be undefined.

Of course the change won't harm, but encouraging this use is not really a good idea, I think.

[Remy Blank]

Replying to cboos:

Of course the change won't harm, but encouraging this use is not really a good idea, I think.

It's not documented, so we can hardly say that we're encouraging it. And technically, it's still undefined, as we return the value we get from Cursor.execute() instead of just self. I just re-established the previous undefined behavior ;-)

But feel free to revert if you think this gives a bad example.

[osimons]

And, Christian: Remember the need to reset the version number on the branch after releases…? ;-)

[Christian Boos]

Well, I should probably have told Remy about my desire to not commit anymore on 0.11-stable except if we have changes that would justify a new release…

[Remy Blank]

Replying to cboos:

Well, I should probably have told Remy about my desire to not commit anymore on 0.11-stable except if we have changes that would justify a new release…

That would have helped, yes. However, see comment:6:ticket:8884. Not really critical, but quite serious, I would say.

OTOH, as long as everything is merged to trunk, I don't see the harm in having unreleased changesets on 0.11-stable.