Обсуждение: psycopg2.connect change from a C function to module method

Hi,

the change that made psycopg2.connect a module-level Python function
rather than a function exposed from a C module turned ou to be
backwards-incompatible.

Attached is a small snippet that works well with psycopg2 2.4.2 and
tracebacks with "TypeError: argument 1 must be string, not C" with 2.4.3.

The potential for actual breakage is very small, but I wanted to report
it in case someone hits it like I did and perhaps to discuss whether the
fix I applied is correct.

This was first reported to me by users of txpostgres, which got broken
by the connect() changes. The fix I applied is:

https://github.com/wulczer/txpostgres/commit/b9ffbbd72cff261da5d37d76a2c1e9f099848014

Cheers,
Jan

On 27/12/11 00:29, Jan Urbański wrote:
> the change that made psycopg2.connect a module-level Python function
> rather than a function exposed from a C module turned ou to be
> backwards-incompatible.
>
> Attached is a small snippet that works well with psycopg2 2.4.2 and
> tracebacks with "TypeError: argument 1 must be string, not C" with 2.4.3.
>
> The potential for actual breakage is very small, but I wanted to report
> it in case someone hits it like I did and perhaps to discuss whether the
> fix I applied is correct.
>
> This was first reported to me by users of txpostgres, which got broken
> by the connect() changes. The fix I applied is:
>
> https://github.com/wulczer/txpostgres/commit/b9ffbbd72cff261da5d37d76a2c1e9f099848014

The fix seems correct to me. I guess if it is possible to tell Python
that the connect function shouldn't be treated as an instance method
directly in psycopg2.py.

federico

2011/12/27 Jan Urbański <wulczer@wulczer.org>:
> Hi,
>
> the change that made psycopg2.connect a module-level Python function
> rather than a function exposed from a C module turned ou to be
> backwards-incompatible.
>
> Attached is a small snippet that works well with psycopg2 2.4.2 and
> tracebacks with "TypeError: argument 1 must be string, not C" with 2.4.3.

Uhm... if you assign a function to a class you get an unbound method:
this is the standard Python semantic. The fact it doesn't happen with
a C function seems just short of a cpython bug, and it's an ugly
asymmetry anyway.


> The potential for actual breakage is very small, but I wanted to report
> it in case someone hits it like I did and perhaps to discuss whether the
> fix I applied is correct.

I would have probably guarded it with an "if isinstance(conn,
types.UnboundMethodType)": because a C function doesn't become a
method, being able to make a staticmethod out of it seems a bet.

Even better, probably:

    class C(object):
        def conn(self, *args, **kwargs):
            return psycopg2.connect(*args, **kwargs)

        def __init__(self):
            self.conn('')

to give subclasses the possibility to change it in a standard OOP way.


-- Daniele

On 28/12/11 13:26, Daniele Varrazzo wrote:
> 2011/12/27 Jan Urbański <wulczer@wulczer.org>:
>> Attached is a small snippet that works well with psycopg2 2.4.2 and
>> tracebacks with "TypeError: argument 1 must be string, not C" with 2.4.3.
>
> Uhm... if you assign a function to a class you get an unbound method:
> this is the standard Python semantic. The fact it doesn't happen with
> a C function seems just short of a cpython bug, and it's an ugly
> asymmetry anyway.

Yeah, I couldn't find any real explanation of whether that's intended
behaviour or a bug.

> Even better, probably:
>
>     class C(object):
>         def conn(self, *args, **kwargs):
>             return psycopg2.connect(*args, **kwargs)
>
>         def __init__(self):
>             self.conn('')
>
> to give subclasses the possibility to change it in a standard OOP way.

That would be nice, but I can't do that because of
backwards-compatibility (overriding the connectionFactory class variable
needs to work).

I'll stick with staticmethod for now and if situation warrants (like for
instance CPython decides you can't call staticmethod on a C function)
I'll turn to something more elaborate.

Thanks!
Jan