Обсуждение: Feature branches merged to master for 2.8 release
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
Hi Daniele,
what about the "_get_native_connection()" ?
Regards
Ch.F.
-------------------------------------------------------------
Good design can't stop bad implementation
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
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
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
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
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
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
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
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
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
> 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
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: >
> 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.
> 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.
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
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.
> 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.
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
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
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
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
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
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