Thread: Feature branches merged to master for 2.8 release

Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
Hello,

I've merged most of what I'd like to release for 2.8 into master, so
that I could generate the whole documentation for you to have a look.
The "what's new" [1] in the docs has all the references:

[1] http://initd.org/psycopg/docs/news.html#what-s-new-in-psycopg-2-8

The features are not necessarily finalised in their current forms, so
I'd like to hear from you if you think improvements can be made. The
main points are:

- the new 'errors' module. About it I have a doubt: we convert
postgres error messages [2] from lower_case to CamelCase because the
latter is the convention for Python class, but maybe leaving as they
are makes more sense? Easier to google for them or grep for them in
the postgres sources maybe?
[2] https://www.postgresql.org/docs/current/static/errcodes-appendix.html

- the new properties on 'cursor.description': does it all make sense?

- the new 'connection.info' object: are there other info we could add
or move there? Maybe the new-ish get_dsn_parameters()?

- Anything else?

On the plate for release there is still a couple of small features
(#591: adapting IntEnum right and #782: the capsules) and a bug (#788:
adapting nested empty arrays - aka "the postgres array parser hates
you"). If there is any taker for them you are welcome.

Comments?

Cheers!

-- Daniele


Re: Feature branches merged to master for 2.8 release

From
Christian Ferrari
Date:
Hi Daniele,
what about the "_get_native_connection()" ?
Regards
Ch.F.

-------------------------------------------------------------
Good design can't stop bad implementation


Il lunedì 15 ottobre 2018, 13:48:24 CEST, Daniele Varrazzo <daniele.varrazzo@gmail.com> ha scritto:


Hello,

I've merged most of what I'd like to release for 2.8 into master, so
that I could generate the whole documentation for you to have a look.
The "what's new" [1] in the docs has all the references:


The features are not necessarily finalised in their current forms, so
I'd like to hear from you if you think improvements can be made. The
main points are:

- the new 'errors' module. About it I have a doubt: we convert
postgres error messages [2] from lower_case to CamelCase because the
latter is the convention for Python class, but maybe leaving as they
are makes more sense? Easier to google for them or grep for them in
the postgres sources maybe?

- the new properties on 'cursor.description': does it all make sense?

- the new 'connection.info' object: are there other info we could add
or move there? Maybe the new-ish get_dsn_parameters()?

- Anything else?

On the plate for release there is still a couple of small features
(#591: adapting IntEnum right and #782: the capsules) and a bug (#788:
adapting nested empty arrays - aka "the postgres array parser hates
you"). If there is any taker for them you are welcome.

Comments?

Cheers!

-- Daniele

Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Mon, Oct 15, 2018 at 1:00 PM Christian Ferrari <camauz@yahoo.com> wrote:
>
> Hi Daniele,
> what about the "_get_native_connection()" ?

That's the "capsule", and yes, I'd like to have it in 2.8. I think
Federico has already code in a branch. I think we are only wondering
whether to add one on the cursor to retrieve the native result. I
think we should: it would have been useful e.g. in cases like #661. We
have now exposing a way to retrieve the requested info but, if we had
had the capsule with the PGresult pointer, the OP could have used
ctypes or cffi to use the function from the libpq in the meantime.

-- Daniele


Re: Feature branches merged to master for 2.8 release

From
Karsten Hilbert
Date:
On Mon, Oct 15, 2018 at 12:48:04PM +0100, Daniele Varrazzo wrote:

> - the new 'errors' module. About it I have a doubt: we convert
> postgres error messages [2] from lower_case to CamelCase because the
> latter is the convention for Python class, but maybe leaving as they
> are makes more sense? Easier to google for them or grep for them in
> the postgres sources maybe?

Since there is no Right or Wrong here, the "best" option
might be to offer both:

Raise whatever is Right for Python (that is, CamelCase)

    raise PostgresSpecificError

but support catching lower case, too:

    class postgres_specific_error(PostgresSpecificError):
        pass

    try:
        something()
    except postgres_specific_error:
        print('lower case')

For the lower case one I would exactly copy what's used by
PostgreSQL itself.

Make sense ?

Karsten
-- 
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B


Re: Feature branches merged to master for 2.8 release

From
Federico Di Gregorio
Date:
On 10/15/2018 02:09 PM, Daniele Varrazzo wrote:
> On Mon, Oct 15, 2018 at 1:00 PM Christian Ferrari<camauz@yahoo.com>  wrote:
>> Hi Daniele,
>> what about the "_get_native_connection()" ?
> That's the "capsule", and yes, I'd like to have it in 2.8. I think
> Federico has already code in a branch. I think we are only wondering
> whether to add one on the cursor to retrieve the native result. I
> think we should: it would have been useful e.g. in cases like #661. We
> have now exposing a way to retrieve the requested info but, if we had
> had the capsule with the PGresult pointer, the OP could have used
> ctypes or cffi to use the function from the libpq in the meantime.

Yes, I'll merge that tomorrow (no time today). I was for a name without 
the underscore but the MySQL guy has a point where he says that the 
client code should be warned that the driver can't guarantee API 
stability in this case (what if PGconn changes?). So, maybe 
`_get_native_connection()` is OK.

federico

-- 
Federico Di Gregorio                         federico.digregorio@dndg.it
DNDG srl                                                  http://dndg.it
  Il panda ha l'apparato digerente di un carnivoro (e.g., di un orso).
   Il panda ha scelto di cibarsi esclusivamente di germogli di bambù.
   Quindi, il panda è l'unico animale vegano del pianeta. Il panda
   merita di estinguersi.                       -- Maria, Alice, Federico


Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Mon, Oct 15, 2018 at 1:12 PM Karsten Hilbert <Karsten.Hilbert@gmx.net> wrote:
>
> On Mon, Oct 15, 2018 at 12:48:04PM +0100, Daniele Varrazzo wrote:
>
> > - the new 'errors' module. About it I have a doubt: we convert
> > postgres error messages [2] from lower_case to CamelCase because the
> > latter is the convention for Python class, but maybe leaving as they
> > are makes more sense? Easier to google for them or grep for them in
> > the postgres sources maybe?
>
> Since there is no Right or Wrong here, the "best" option
> might be to offer both:
>
> Raise whatever is Right for Python (that is, CamelCase)
>
>         raise PostgresSpecificError
>
> but support catching lower case, too:
>
>         class postgres_specific_error(PostgresSpecificError):
>                 pass
>
>         try:
>                 something()
>         except postgres_specific_error:
>                 print('lower case')
>
> For the lower case one I would exactly copy what's used by
> PostgreSQL itself.
>
> Make sense ?

I think it's responsible to make a decision. Having two names to refer
to a thing is confusing. One of the two would be the "blessed" one -
the one appearing in the tracebacks. And if a traceback says that
'unique_violation' was raised, it would be weird to solve it by
writing a handler for 'UniqueViolation'.

Plus, that module defines already 232 classes (as of Postgres 11): I
wouldn't like doubling their number. I'm pondering whether to write it
as a C module instead of Python but I'm not overly fussed by that.

So I'd say lower_case or CamelCase, not both.


-- Daniele


Re: Feature branches merged to master for 2.8 release

From
Federico Di Gregorio
Date:
On 10/15/2018 07:23 PM, Daniele Varrazzo wrote:
> On Mon, Oct 15, 2018 at 1:12 PM Karsten Hilbert<Karsten.Hilbert@gmx.net>  wrote:
>> On Mon, Oct 15, 2018 at 12:48:04PM +0100, Daniele Varrazzo wrote:
>>
>>> - the new 'errors' module. About it I have a doubt: we convert
>>> postgres error messages [2] from lower_case to CamelCase because the
>>> latter is the convention for Python class, but maybe leaving as they
>>> are makes more sense? Easier to google for them or grep for them in
>>> the postgres sources maybe?
>> Since there is no Right or Wrong here, the "best" option
>> might be to offer both:
>>
>> Raise whatever is Right for Python (that is, CamelCase)
>>
>>          raise PostgresSpecificError
>>
>> but support catching lower case, too:
>>
>>          class postgres_specific_error(PostgresSpecificError):
>>                  pass
>>
>>          try:
>>                  something()
>>          except postgres_specific_error:
>>                  print('lower case')
>>
>> For the lower case one I would exactly copy what's used by
>> PostgreSQL itself.
>>
>> Make sense ?
> I think it's responsible to make a decision. Having two names to refer
> to a thing is confusing. One of the two would be the "blessed" one -
> the one appearing in the tracebacks. And if a traceback says that
> 'unique_violation' was raised, it would be weird to solve it by
> writing a handler for 'UniqueViolation'.
> 
> Plus, that module defines already 232 classes (as of Postgres 11): I
> wouldn't like doubling their number. I'm pondering whether to write it
> as a C module instead of Python but I'm not overly fussed by that.
> 
> So I'd say lower_case or CamelCase, not both.

Python exceptions are CamelCase in 99% of the cases (pun intended).
Lets not have every single Python programmer out there hate us because 
we decided to import a database backend convention into a programming 
laguage.

federico

-- 
Federico Di Gregorio                         federico.digregorio@dndg.it
DNDG srl                                                  http://dndg.it
               Una nazionale senza neanche una nazione. -- macchinavapore


Re: Feature branches merged to master for 2.8 release

From
Karsten Hilbert
Date:
On Mon, Oct 15, 2018 at 06:23:57PM +0100, Daniele Varrazzo wrote:

> I think it's responsible to make a decision. Having two names to refer
> to a thing is confusing. One of the two would be the "blessed" one -
> the one appearing in the tracebacks. And if a traceback says that
> 'unique_violation' was raised, it would be weird to solve it by
> writing a handler for 'UniqueViolation'.

I would then stick with UniqueViolation.

Perhaps an attribute

    UniqueViolation.pg_error_name = unique_violation

so people _can_ know what to grep for ?

Karsten
-- 
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B


Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Mon, Oct 15, 2018 at 6:36 PM Federico Di Gregorio <fog@dndg.it> wrote:

> Python exceptions are CamelCase in 99% of the cases (pun intended).
> Lets not have every single Python programmer out there hate us because
> we decided to import a database backend convention into a programming
> laguage.

Yes, that's how I started implementing it that way. Thank you for the
feedback :)

-- Daniele


Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Mon, Oct 15, 2018 at 6:43 PM Karsten Hilbert <Karsten.Hilbert@gmx.net> wrote:

> I would then stick with UniqueViolation.
>
> Perhaps an attribute
>
>         UniqueViolation.pg_error_name = unique_violation
>
> so people _can_ know what to grep for ?

It makes sense of course. And I was hoping it was available in the
diag object [1]. But apparently it's not... So good call: I'll try to
add it to the classes.

[1] http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.Diagnostics

-- Daniele


Python vers, was 2.8 feature branch

From
"Brian M Hamlin"
Date:
 
> Lets not have every single Python programmer out there hate us
> because we decided to import a database backend convention into a
> programming language.
 
sadly, I had another conversation recently with an engineering
architect in San Francisco..
Their company has implemented a lot of data pipeline in Python 2.7, and
continues to build and maintain it.. the younger engineers became enraged
at the choice and indulged in mild harassment recently, because of it ..
 
The sense of maintaining a working pipeline, and the cost of porting it
due to the fashions of the times, are not in sync with the emotional
appeals to build in Python 3.x. The company has no plans to remove the
working Python 2.7 pipeline.
 
side-note:
The Debian/Ubuntu 1804 OSGeo Linux setup is built with complete Python 2.7
libraries, and I believe it is the best Python 2.7 to date.  I
regularly have to
defend this against drive-by disparagement, yet this system works very well.
 
FYI

--
Brian M Hamlin
OSGeo California
blog.light42.com

 



Re: Feature branches merged to master for 2.8 release

From
Aryeh Leib Taurog
Date:
I also think CamelCase should be preferred for consistency with
accepted Python standards.  It seems to me that if the exception
string representation includes the PostgreSQL snake_case name (perhaps
in parentheses), that should be sufficient for any reference needs,
because it will appear in the message printed with the traceback.
Usually if I want to search for more information about an error, I
just paste that into my browser.

Also, I wouldn’t be surprised if google is smart enough to handle the
switch between camel case and snake case, but I haven’t tested it.

In any case, this is an exciting and welcome addition to an already
excellent library.  Thanks much for all your efforts.


> On Mon, Oct 15, 2018 at 06:23:57PM +0100, Daniele Varrazzo wrote:
>


Re: Feature branches merged to master for 2.8 release

From
Christian Ferrari
Date:

> Yes, I'll merge that tomorrow (no time today). I was for a name without
> the underscore but the MySQL guy has a point where he says that the
> client code should be warned that the driver can't guarantee API
> stability in this case (what if PGconn changes?). So, maybe
> `_get_native_connection()` is OK.

> federico

Dear Federico,
mysqlclient-python accepted the pull request
and the final method name for MySQLdb connection is "_get_native_connection()"

I'm ready to release LIXA 1.7.1 with XTA support for Python2/3 and PostgreSQL/MySQL/MariaDB, but currently I still have all the examples with
"get_native_connection()" for PostgreSQL
and
"_get_native_connection()" for MySQL/MariaDB

Do you mind if I fork the master branch of Psycopg2 and apply your changes? I would like to avoid a release with inconsistent method name that should be fixed as soon as Psycopg2 2.8 will be available.

Kind Regards
Ch.F.



Re: Feature branches merged to master for 2.8 release

From
Christian Ferrari
Date:
Dear All,
today I have released LIXA 1.7.1 with Python/Psycopg2 support.
To avoid two different methods (get_native_connection() for PostgreSQL and _get_native_connection() for MySQL/MariaDB) I have created a fork of Psycopg2 https://github.com/tiian/psycopg2/tree/get-native-connection with the code provided by Federico and method "get_native_connection" renamed.
For the sake of convenience I have created a pull request: https://github.com/psycopg/psycopg2/pull/798 feel free to integrate it or to go on with the process you figured out in the past.
Kind Regards
Ch.F.

-------------------------------------------------------------
Good design can't stop bad implementation


Il domenica 21 ottobre 2018, 22:05:14 CEST, Christian Ferrari <camauz@yahoo.com> ha scritto:



> Yes, I'll merge that tomorrow (no time today). I was for a name without
> the underscore but the MySQL guy has a point where he says that the
> client code should be warned that the driver can't guarantee API
> stability in this case (what if PGconn changes?). So, maybe
> `_get_native_connection()` is OK.

> federico

Dear Federico,
mysqlclient-python accepted the pull request
and the final method name for MySQLdb connection is "_get_native_connection()"

I'm ready to release LIXA 1.7.1 with XTA support for Python2/3 and PostgreSQL/MySQL/MariaDB, but currently I still have all the examples with
"get_native_connection()" for PostgreSQL
and
"_get_native_connection()" for MySQL/MariaDB

Do you mind if I fork the master branch of Psycopg2 and apply your changes? I would like to avoid a release with inconsistent method name that should be fixed as soon as Psycopg2 2.8 will be available.

Kind Regards
Ch.F.



Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Sun, Oct 28, 2018 at 9:35 PM Christian Ferrari <camauz@yahoo.com> wrote:
>
> Dear All,
> today I have released LIXA 1.7.1 with Python/Psycopg2 support.
> To avoid two different methods (get_native_connection() for PostgreSQL and _get_native_connection() for
MySQL/MariaDB)I have created a fork of Psycopg2 https://github.com/tiian/psycopg2/tree/get-native-connection with the
codeprovided by Federico and method "get_native_connection" renamed. 
> For the sake of convenience I have created a pull request: https://github.com/psycopg/psycopg2/pull/798 feel free to
integrateit or to go on with the process you figured out in the past. 
> Kind Regards
> Ch.F.

I don't know what Federico had decided but I'd very much prefer the
name without underscore. Methods with underscore don't belong to an
object interface.

There are always differences, in interfaces and in behaviour, between
dbapi drivers. A private method, returning two objects which are
different because they are very simply two database drivers, are not
the place where to start trying to unify interfaces. What you do, if
you want to deal with both drivers, is to write a wrapper unifying the
interfaces. I would be ok to adapt other drivers choices, if I agreed
with them, but I can't agree with an underscore method being
documented as part of the API: that's internal stuff and totally not
idiomatic Python.

My vote goes to have this method name without underscore, but I'll
leave the final decision to Federico.

-- Daniele


Re: Feature branches merged to master for 2.8 release

From
Karsten Hilbert
Date:
On Mon, Oct 29, 2018 at 10:49:14AM +0000, Daniele Varrazzo wrote:

> There are always differences, in interfaces and in behaviour, between
> dbapi drivers. A private method, returning two objects which are
> different because they are very simply two database drivers, are not
> the place where to start trying to unify interfaces. What you do, if
> you want to deal with both drivers, is to write a wrapper unifying the
> interfaces. I would be ok to adapt other drivers choices, if I agreed
> with them, but I can't agree with an underscore method being
> documented as part of the API: that's internal stuff and totally not
> idiomatic Python.
> 
> My vote goes to have this method name without underscore, but I'll
> leave the final decision to Federico.

Apart from a wrapper class one can always monkey-patch

    my_dbapi_instance._get_native_connection = my_dbapi_instance.get_native_connection

at runtime.

Karsten
-- 
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B


Re: Feature branches merged to master for 2.8 release

From
Federico Di Gregorio
Date:
On 10/29/2018 11:49 AM, Daniele Varrazzo wrote:
> On Sun, Oct 28, 2018 at 9:35 PM Christian Ferrari <camauz@yahoo.com> wrote:
>>
>> Dear All,
>> today I have released LIXA 1.7.1 with Python/Psycopg2 support.
>> To avoid two different methods (get_native_connection() for PostgreSQL and _get_native_connection() for
MySQL/MariaDB)I have created a fork of Psycopg2 https://github.com/tiian/psycopg2/tree/get-native-connection with the
codeprovided by Federico and method "get_native_connection" renamed.
 
>> For the sake of convenience I have created a pull request: https://github.com/psycopg/psycopg2/pull/798 feel free to
integrateit or to go on with the process you figured out in the past.
 
>> Kind Regards
>> Ch.F.
> 
> I don't know what Federico had decided but I'd very much prefer the
> name without underscore. Methods with underscore don't belong to an
> object interface.

Sorry for the late response, I was KO'ed by the flu. I agree with 
Daniele here but the fact that the *content* of the capsule, i.e., the 
PGconn* can change depending of the libpq version you're loading is 
pretty scary. I'd like to warn the client code (Python warnings to be 
suppressed? mm..) but just prepending an underscore to the method name 
doesn't mean much. If we add `get_native_connection()` then it becomes 
an official and supported psycopg extensions and doesn't make sense to 
mark it as private API.

> There are always differences, in interfaces and in behaviour, between
> dbapi drivers. A private method, returning two objects which are
> different because they are very simply two database drivers, are not
> the place where to start trying to unify interfaces. What you do, if
> you want to deal with both drivers, is to write a wrapper unifying the
> interfaces. I would be ok to adapt other drivers choices, if I agreed
> with them, but I can't agree with an underscore method being
> documented as part of the API: that's internal stuff and totally not
> idiomatic Python.
> 
> My vote goes to have this method name without underscore, but I'll
> leave the final decision to Federico.

Then without it is.

federico

-- 
Federico Di Gregorio                         federico.digregorio@dndg.it
DNDG srl                                                  http://dndg.it
  Credo fermamente che da qualche parte, in una scatola ci sia un gatto
   che non è vivo ne morto. Credo anche che se i fisici non si sbrigano
   a dargli una scatoletta, ben presto sarà solo morto.
                               -- adattato da una frase di Sam Black Crow


Re: Feature branches merged to master for 2.8 release

From
Federico Di Gregorio
Date:
On 10/29/2018 11:54 AM, Karsten Hilbert wrote:
> On Mon, Oct 29, 2018 at 10:49:14AM +0000, Daniele Varrazzo wrote:
> 
>> There are always differences, in interfaces and in behaviour, between
>> dbapi drivers. A private method, returning two objects which are
>> different because they are very simply two database drivers, are not
>> the place where to start trying to unify interfaces. What you do, if
>> you want to deal with both drivers, is to write a wrapper unifying the
>> interfaces. I would be ok to adapt other drivers choices, if I agreed
>> with them, but I can't agree with an underscore method being
>> documented as part of the API: that's internal stuff and totally not
>> idiomatic Python.
>>
>> My vote goes to have this method name without underscore, but I'll
>> leave the final decision to Federico.
> Apart from a wrapper class one can always monkey-patch
> 
>     my_dbapi_instance._get_native_connection = my_dbapi_instance.get_native_connection
> 
> at runtime.

Right.

federico

-- 
Federico Di Gregorio                         federico.digregorio@dndg.it
DNDG srl                                                  http://dndg.it
  But not all bugs are an interesting challenge. Some are just a total
   waste of my time, which usually is much more valuable than the time of
   the submitter.                                                   -- Md


Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Mon, Oct 29, 2018 at 11:47 AM Federico Di Gregorio <fog@dndg.it> wrote:

> I agree with
> Daniele here but the fact that the *content* of the capsule, i.e., the
> PGconn* can change depending of the libpq version you're loading is
> pretty scary. I'd like to warn the client code (Python warnings to be
> suppressed? mm..) but just prepending an underscore to the method name
> doesn't mean much. If we add `get_native_connection()` then it becomes
> an official and supported psycopg extensions and doesn't make sense to
> mark it as private API.

The PGconn is an opaque structure: its correct usage is through the
libpq functions. See also the note in
<https://www.postgresql.org/docs/current/static/libpq-status.html>:
"libpq application programmers should be careful to maintain the
PGconn abstraction. Use the accessor functions described below to get
at the contents of PGconn. Reference to internal PGconn fields using
libpq-int.h is not recommended because they are subject to change in
the future." If the structure changes and someone gets bitten by it
it's not really related to how much private the method is.


-- Daniele


Re: Feature branches merged to master for 2.8 release

From
Daniele Varrazzo
Date:
On Mon, Oct 29, 2018 at 10:54 AM Karsten Hilbert
<Karsten.Hilbert@gmx.net> wrote:

> Apart from a wrapper class one can always monkey-patch
>
>         my_dbapi_instance._get_native_connection = my_dbapi_instance.get_native_connection
>
> at runtime.

True for a Python object, but not for a C extension object. But
subclassing is supported, so one would be able to do:

    >>> class MyConnection(psycopg2.extensions.connection):
    ...     _get_native_connection =
psycopg2.extensions.connection.get_native_connection
    ...
    >>> cnn = psycopg2.connect('', connection_factory=MyConnection)
    >>> ptr = cnn._get_native_connection()

-- Daniele