Thread: RFC: Extend psycopg2.connect to accept all valid parameters?

RFC: Extend psycopg2.connect to accept all valid parameters?

From
Fabian Knittel
Date:
Hello everyone,

I'm trying to pass the "sslrootcert" connection keyword parameter
through sqlalchemy and psycopg2.  Unfortunately, that's currently not
possible in a straightforward way, meaning, this sqlalchemy URI doesn't
work:


postgresql://<username>@<hostname>:<port>/<dbname>?sslmode=verify-full&sslrootcert=<cert>

The sqlalchemy dialect for psycopg2 passes all parameters as kwargs
(instead of as dsn string) to psycopg2.connect, but psycopg2.connect
only supports a subset [1] of all valid parameter keywords [2], so
"sslrootcert" isn't accepted.

There's a work-around [3], but it's ugly.  So now I'd like to fix it and
I'm trying to find out whether to change psycopg2 or sqlalchemy.

Properly fixing it in psycopg2 would probably mean accepting all missing
keywords [4] in psycopg2.connect.

At this point I'm interested in what kind of patch would be acceptable.
The options I came up with:
 a) A patch to explicitly support all keywords that are currently
    supported by the newest libpq.  (psycopg2 would need to be changed
    for future keywords.)
 b) A patch to accept and pass-on any keyword parameters.
 c) "None of the above, go fix sqlalchemy to use a dsn string instead
    of keyword parameters."

Cheers
Fabian

1: Supported parameters: dbname, database, user, password, host, port,
sslmode
2:
http://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-PQCONNECTDBPARAMS
3: Work-around: Using psycopg2.connect's undocumented keyword parameter
'dsn' through an sqlalchemy URI such as

"postgresql:///?dsn=user=<username>%20host=<hostname>%20port=<port>%20dbname=<dbname>%20sslmode=verify-full%20sslrootcert=<cert>"
4: Currently missing keywords: hostaddr, connect_timeout,
client_encoding, options, application_name, fallback_application_name,
keepalives, keepalives_idle, keepalives_interval, keepalives_count,
sslcert, sslkey, sslrootcert, sslcrl, requirepeer, krbsrvname, gsslib,
service. (I've omitted requiressl and tty, because they are
deprecated/ignored.)


Attachment

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
Hello Fabian,

On Wed, Nov 16, 2011 at 5:56 PM, Fabian Knittel
<fabian.knittel@avona.com> wrote:

> I'm trying to pass the "sslrootcert" connection keyword parameter
> through sqlalchemy and psycopg2.  Unfortunately, that's currently not
> possible in a straightforward way, meaning, this sqlalchemy URI doesn't
> work:
>
>
> postgresql://<username>@<hostname>:<port>/<dbname>?sslmode=verify-full&sslrootcert=<cert>

os.environ['PGSSLROOTCERT'] is (should be) your friend. Also see
<http://www.postgresql.org/docs/9.1/static/libpq-envars.html>.


> Properly fixing it in psycopg2 would probably mean accepting all missing
> keywords [4] in psycopg2.connect.

> 4: Currently missing keywords: hostaddr, connect_timeout,
> client_encoding, options, application_name, fallback_application_name,
> keepalives, keepalives_idle, keepalives_interval, keepalives_count,
> sslcert, sslkey, sslrootcert, sslcrl, requirepeer, krbsrvname, gsslib,
> service. (I've omitted requiressl and tty, because they are
> deprecated/ignored.)

As you see they are a lot, and counting. If I'm not mistaken there is
a fallback env variable for each of them. I've used application_name
sometimes.


-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Fabian Knittel
Date:
Hello Daniele,

Am 16.11.2011 19:56, schrieb Daniele Varrazzo:
> On Wed, Nov 16, 2011 at 5:56 PM, Fabian Knittel
> <fabian.knittel@avona.com> wrote:
>> I'm trying to pass the "sslrootcert" connection keyword parameter
>> through sqlalchemy and psycopg2.
[...]
> os.environ['PGSSLROOTCERT'] is (should be) your friend. Also see
> <http://www.postgresql.org/docs/9.1/static/libpq-envars.html>.

True, this is another possible approach.  I personally don't see it as
viable solution, because it's not easy to configure via a configuration
file and you can't really configure multiple connections.  In my
applications you currently select and configure all DB backends by
setting apropriate connection URIs in the configuration file.  Using
environment variables to configure some aspects of a backend would need
explicit code to set and unset the options when using the PostgreSQL
backend (carefully setting it right before a specific connection and
resetting it for the next connection with a different setting).  This is
possible, but IMHO falls into the work-around category.  I'd probably
use the dsn-work-around mentioned in my initial mail before resorting to
environment variables.

>> Properly fixing it in psycopg2 would probably mean accepting all missing
>> keywords [4] in psycopg2.connect.
>
>> 4: Currently missing keywords: hostaddr, connect_timeout,
>> client_encoding, options, application_name, fallback_application_name,
>> keepalives, keepalives_idle, keepalives_interval, keepalives_count,
>> sslcert, sslkey, sslrootcert, sslcrl, requirepeer, krbsrvname, gsslib,
>> service. (I've omitted requiressl and tty, because they are
>> deprecated/ignored.)
>
> As you see they are a lot, and counting. If I'm not mistaken there is
> a fallback env variable for each of them. [...]

(Sorry, I should have sorted my proposed solutions.)

I agree that option a) is probably the least desirable one.  But what do
you think of option b)?  Effectively, it would be something along the
lines of

  dsn = ' '.join(
      ['%s=%s' % (key, str(val)) for key, val in kwargs.iteritems()])

translated to C (with a few additional translations, e.g. database to
dbname).  All keyword arguments would be translated to a connection
string, without explicit checking.  Any unknown keywords would then be
detected by libpq the same way they are detected when the dsn is
explicitly given.

Cheers
Fabian


Attachment

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Wed, Nov 16, 2011 at 7:39 PM, Fabian Knittel
<fabian.knittel@avona.com> wrote:
> Hello Daniele,
>
> Am 16.11.2011 19:56, schrieb Daniele Varrazzo:
>> On Wed, Nov 16, 2011 at 5:56 PM, Fabian Knittel
>> <fabian.knittel@avona.com> wrote:
>>> I'm trying to pass the "sslrootcert" connection keyword parameter
>>> through sqlalchemy and psycopg2.
> [...]
>> os.environ['PGSSLROOTCERT'] is (should be) your friend. Also see
>> <http://www.postgresql.org/docs/9.1/static/libpq-envars.html>.
>
> True, this is another possible approach.  I personally don't see it as
> viable solution, because it's not easy to configure via a configuration
> file and you can't really configure multiple connections.

You are right, env variables are less than optimal. I forgot a better
option: if connect() is called with a single string parameter instead
of keyword arguments you can pass any libpq parameter:

    In [16]: cnn = psycopg2.connect('host=localhost application_name=foo')
    In [17]: cur = cnn.cursor()
    In [19]: cur.execute("select application_name from pg_stat_activity")
    In [20]: cur.fetchall()
    Out[20]: [('foo',)]

I also see that the libpq checks if the keywords is really supported:

    In [21]: cnn = psycopg2.connect('host=localhost bax=qux')
    ---------------------------------------------------------------------------
    OperationalError                          Traceback (most recent call last)

    /home/piro/dev/psycopg2/build/lib.2.7/<ipython console> in <module>()

    OperationalError: invalid connection option "bax"

This is definitely a better solution than the env.


> I agree that option a) is probably the least desirable one.  But what do
> you think of option b)?  Effectively, it would be something along the
> lines of
>
>  dsn = ' '.join(
>      ['%s=%s' % (key, str(val)) for key, val in kwargs.iteritems()])
>
> translated to C (with a few additional translations, e.g. database to
> dbname).  All keyword arguments would be translated to a connection
> string, without explicit checking.  Any unknown keywords would then be
> detected by libpq the same way they are detected when the dsn is
> explicitly given.

Yes, I'd say having a pass-through **kwargs from the connect()
parameters to the PQconnect string is the best solution; psycopg
doesn't really need to know all of them and the error message raised
by the libpq in case of errors is perfectly explicit.

I don't see anything bad against the feature. But: do we really need
the keywords pass-through, given the fact that the connection string
already covers all the required cases? It looks more like syntactic
sugar, and if you have to fix a client to support a new keyword / all
the keywords, they can be fixed by generating a connection string
instead of forwarding the **kwargs. ... mmm... reading back the head
of the thread, as you describe it sqlalchemy would already work using
the **kwargs, whereas it should be patched to pass psycopg a
connection string instead. So it's a matter of patching us (to pass
**kwargs through) or them (to convert the url to a connection string
instead of a doct) it seems...

Thoughts? Patches?


-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Fabian Knittel
Date:
Hello Daniele,

Am 16.11.2011 21:50, schrieb Daniele Varrazzo:
> You are right, env variables are less than optimal. I forgot a better
> option: if connect() is called with a single string parameter instead
> of keyword arguments you can pass any libpq parameter:
[...]
> This is definitely a better solution than the env.

Agreed.  Explicitly building the dsn would be a viable solution, which
could be implemented in sqlalchemy.  This is what I was aiming at in
solution c) from my initial mail.  Instead of passing kwargs, sqlalchemy
would need to be changed to explicitly build the dsn string and pass
that to psycopg.

[... Snipped discussion of solution b) ...]
> I don't see anything bad against the feature. But: do we really need
> the keywords pass-through, given the fact that the connection string
> already covers all the required cases? It looks more like syntactic
> sugar, and if you have to fix a client to support a new keyword / all
> the keywords, they can be fixed by generating a connection string
> instead of forwarding the **kwargs. ... mmm... reading back the head
> of the thread, as you describe it sqlalchemy would already work using
> the **kwargs, whereas it should be patched to pass psycopg a
> connection string instead. So it's a matter of patching us (to pass
> **kwargs through) or them (to convert the url to a connection string
> instead of a doct) it seems...

Precisely.  This is what I was trying to discuss. :-) (Sorry for not
clearly communicating that from the start.)

I'm willing to write a patch, but I first wanted to find out what kind
of patch (if any) would be acceptable in psycopg.  The pass-through
solution is clearly only syntactic sugar, but as there's already some
keyword support, it would be nice to make this more generic and
therefore more intuitive.

Instead of implementing the kwargs-pass-through approach in C, I could
also imagine a solution where psycopg2._psycopg.connect (psyco_connect)
is changed to only support a single string-DSN-parameter.
psycopg2.connect would be a pure-Python method that wraps around
psycopg2._psycopg.connect and provides the kwargs to dsn conversion
sugar.  (This would move all the icky string manipulation stuff from C
to Python.)

Cheers
Fabian


Attachment

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Wed, Nov 16, 2011 at 9:38 PM, Fabian Knittel
<fabian.knittel@avona.com> wrote:

> I'm willing to write a patch, but I first wanted to find out what kind
> of patch (if any) would be acceptable in psycopg.  The pass-through
> solution is clearly only syntactic sugar, but as there's already some
> keyword support, it would be nice to make this more generic and
> therefore more intuitive.

For me, transparent pass-through of the keywords arguments to a
connection string would be perfectly acceptable; an exhaustive list of
libpq-supported parameters not so much.


> Instead of implementing the kwargs-pass-through approach in C, I could
> also imagine a solution where psycopg2._psycopg.connect (psyco_connect)
> is changed to only support a single string-DSN-parameter.
> psycopg2.connect would be a pure-Python method that wraps around
> psycopg2._psycopg.connect and provides the kwargs to dsn conversion
> sugar.  (This would move all the icky string manipulation stuff from C
> to Python.)

Yes, I agree: this is a less scary implementation if you wanted to
provide a patch. I'd either leave psycopg2._psycopg.connect as it is,
with the currently supported keyword arguments, or rename it to
psycopg2._psycopg._connect, supporting only the connection string.
Either way, the function would be imported in the module as _connect,
to be invoked by a connect() function written in python and
responsible to build the connection string.


-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Wed, Nov 16, 2011 at 10:23 PM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:
> On Wed, Nov 16, 2011 at 9:38 PM, Fabian Knittel
> <fabian.knittel@avona.com> wrote:

>> Instead of implementing the kwargs-pass-through approach in C, I could
>> also imagine a solution where psycopg2._psycopg.connect (psyco_connect)
>> is changed to only support a single string-DSN-parameter.
>> psycopg2.connect would be a pure-Python method that wraps around
>> psycopg2._psycopg.connect and provides the kwargs to dsn conversion
>> sugar.  (This would move all the icky string manipulation stuff from C
>> to Python.)
>
> Yes, I agree: this is a less scary implementation if you wanted to
> provide a patch. I'd either leave psycopg2._psycopg.connect as it is,
> with the currently supported keyword arguments, or rename it to
> psycopg2._psycopg._connect, supporting only the connection string.
> Either way, the function would be imported in the module as _connect,
> to be invoked by a connect() function written in python and
> responsible to build the connection string.

I've implemented what discussed here and pushed in a separate branch:
https://github.com/dvarrazzo/psycopg/commit/d2b67364fd2b0b192342281d24a7e3d0a4909980

It's open for discussion. It is not as tested as the rest of the
library, as there aren't many tests covering connect() in all the
possible ways (as there should be as many databases on the other side
to reply). So it needs some manual testing, or a strategy for
automatic ones, definitely more than the 10 minutes I've tried before
pushing.

I'd like to hear from somebody else (mostly Fog, but anybody else)
before having it merged and released: tests and feedbacks are welcome.

Cheers,

-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Jan Urbański
Date:
On 17/11/11 03:02, Daniele Varrazzo wrote:
> On Wed, Nov 16, 2011 at 10:23 PM, Daniele Varrazzo
> <daniele.varrazzo@gmail.com>  wrote:
> I've implemented what discussed here and pushed in a separate branch:
> https://github.com/dvarrazzo/psycopg/commit/d2b67364fd2b0b192342281d24a7e3d0a4909980
>
> It's open for discussion. It is not as tested as the rest of the
> library, as there aren't many tests covering connect() in all the
> possible ways (as there should be as many databases on the other side
> to reply).

I think the correct solution would be to document the 'dsn' parameter to
psycopg2.connect and perhaps even encourage people using it.

This pushes the trouble of parsing out and interpreting parameters to
libpq, which is good, for in the end the important thing is that libpq
sees what you want it to see.

For instance, this patch (and AFAICS vanilla psycopg2 too) does not
handle usernames with spaces in them. Instead of implementing quoting as
expected by libpq, why not just let the user use a DSN that will work
equally with psql as with any other libpq app out there?

As for SQLAlchemy, you can use the connect_args argument to pass a DSN
to create_engine thusly:

e = create_engine('postgresql://', connect_args={'dsn': 'user=foo
sslrootcert=root.crt'})

Cheers,
Jan

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Federico Di Gregorio
Date:
On 17/11/11 03:02, Daniele Varrazzo wrote:
> On Wed, Nov 16, 2011 at 10:23 PM, Daniele Varrazzo
> <daniele.varrazzo@gmail.com> wrote:
>> > On Wed, Nov 16, 2011 at 9:38 PM, Fabian Knittel
>> > <fabian.knittel@avona.com> wrote:
>>> >> Instead of implementing the kwargs-pass-through approach in C, I could
>>> >> also imagine a solution where psycopg2._psycopg.connect (psyco_connect)
>>> >> is changed to only support a single string-DSN-parameter.
>>> >> psycopg2.connect would be a pure-Python method that wraps around
>>> >> psycopg2._psycopg.connect and provides the kwargs to dsn conversion
>>> >> sugar.  (This would move all the icky string manipulation stuff from C
>>> >> to Python.)
>> >
>> > Yes, I agree: this is a less scary implementation if you wanted to
>> > provide a patch. I'd either leave psycopg2._psycopg.connect as it is,
>> > with the currently supported keyword arguments, or rename it to
>> > psycopg2._psycopg._connect, supporting only the connection string.
>> > Either way, the function would be imported in the module as _connect,
>> > to be invoked by a connect() function written in python and
>> > responsible to build the connection string.
> I've implemented what discussed here and pushed in a separate branch:
> https://github.com/dvarrazzo/psycopg/commit/d2b67364fd2b0b192342281d24a7e3d0a4909980
>
> It's open for discussion. It is not as tested as the rest of the
> library, as there aren't many tests covering connect() in all the
> possible ways (as there should be as many databases on the other side
> to reply). So it needs some manual testing, or a strategy for
> automatic ones, definitely more than the 10 minutes I've tried before
> pushing.
>
> I'd like to hear from somebody else (mostly Fog, but anybody else)
> before having it merged and released: tests and feedbacks are welcome.

Looks good but I have a question: does automatic conversion always
generates a valid connection string? Do we need quoting for some of the
parameters? The main idea behind .connect() was to use dsn for the
generic case and provider keyword arguments as a utility for the most
common cases while the patch actually says "it's ok to use kwargs for
everything" and if we do that then it should work out of the box.

But I almost always use username+password only so I don't know if some
parameters need special treatment.

federico

--
Federico Di Gregorio                                       fog@initd.org
 La macchina virtuale elabora quindi dati adempiendo le sue funzioni
  specifiche senza esistere nella realtà degli oggetti.  -- uno studente

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Thu, Nov 17, 2011 at 7:48 AM, Jan Urbański <wulczer@wulczer.org> wrote:
> On 17/11/11 03:02, Daniele Varrazzo wrote:
>>
>> On Wed, Nov 16, 2011 at 10:23 PM, Daniele Varrazzo
>> <daniele.varrazzo@gmail.com>  wrote:
>> I've implemented what discussed here and pushed in a separate branch:
>>
>> https://github.com/dvarrazzo/psycopg/commit/d2b67364fd2b0b192342281d24a7e3d0a4909980
>>
>> It's open for discussion. It is not as tested as the rest of the
>> library, as there aren't many tests covering connect() in all the
>> possible ways (as there should be as many databases on the other side
>> to reply).
>
> I think the correct solution would be to document the 'dsn' parameter to
> psycopg2.connect and perhaps even encourage people using it.
>
> This pushes the trouble of parsing out and interpreting parameters to libpq,
> which is good, for in the end the important thing is that libpq sees what
> you want it to see.
>
> For instance, this patch (and AFAICS vanilla psycopg2 too) does not handle
> usernames with spaces in them.

I told the use of keyword arguments is just syntactic sugar. Which it
is known to cause cancer to the semicolon :)

Jokes aside, yes, I'd say it is a bug in psycopg since day zero. I'd
fix it regardless of the need or not to support generic keywords. Even
though fixing it in python is way easier than fixing in c, so
eventually the fix would lead to a patch very similar to the above
one.

> Instead of implementing quoting as expected
> by libpq, why not just let the user use a DSN that will work equally with
> psql as with any other libpq app out there?

The use of the dsn is documented, users can already use it. I don't
feel like pushing the users really to use either one or the other
parameters style if they both suit, but if you want to suggest
improvement to the documentation, they are welcome.

> As for SQLAlchemy, you can use the connect_args argument to pass a DSN to
> create_engine thusly:
>
> e = create_engine('postgresql://', connect_args={'dsn': 'user=foo
> sslrootcert=root.crt'})

Yes, but these arguments are passed as keywords, not as connection
string. And rightly so I'd say, otherwise it would be their problem to
escape the values building the dsn, and different drivers may have
different escaping rules, making the problem more complex for them. So
having psycopg pass them through correctly escaped as described in
<http://www.postgresql.org/docs/9.1/static/libpq-connect.html#LIBPQ-PQCONNECTDBPARAMS>
looks a good thing.

-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Thu, Nov 17, 2011 at 8:31 AM, Federico Di Gregorio <fog@dndg.it> wrote:

> Looks good but I have a question: does automatic conversion always
> generates a valid connection string? Do we need quoting for some of the
> parameters?

No: just find the docs of the connection string escaping rule. They
are not implemented in psycopg, neither before the patch.

> The main idea behind .connect() was to use dsn for the
> generic case and provider keyword arguments as a utility for the most
> common cases while the patch actually says "it's ok to use kwargs for
> everything" and if we do that then it should work out of the box.
>
> But I almost always use username+password only so I don't know if some
> parameters need special treatment.

PQconnectdb says: To write an empty value, or a value containing
spaces, surround it with single quotes, e.g., keyword = 'a value'.
Single quotes and backslashes within the value must be escaped with a
backslash, i.e., \' and \\.

I'd rather do this in Python than in C, so at this point the patch is
basically what I've written yesterday minus the **kwargs pass-through.
Or, alternatively, a few hundred lines of C with dynamic memory
allocation, tricky conversion of Python objects to byte strings and a
couple of off-by-one errors peppered in less tested code paths...

-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Thu, Nov 17, 2011 at 9:49 AM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:
> On Thu, Nov 17, 2011 at 8:31 AM, Federico Di Gregorio <fog@dndg.it> wrote:
>
>> Looks good but I have a question: does automatic conversion always
>> generates a valid connection string? Do we need quoting for some of the
>> parameters?
>
> No: just find the docs of the connection string escaping rule.

I mean: does automatic conversion always generate a valid dns? No. Do
we need quoting: yes.

-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Federico Di Gregorio
Date:
On 17/11/11 10:49, Daniele Varrazzo wrote:
> On Thu, Nov 17, 2011 at 8:31 AM, Federico Di Gregorio <fog@dndg.it> wrote:
>
>> Looks good but I have a question: does automatic conversion always
>> generates a valid connection string? Do we need quoting for some of the
>> parameters?
>
> No: just find the docs of the connection string escaping rule. They
> are not implemented in psycopg, neither before the patch.
>
>> The main idea behind .connect() was to use dsn for the
>> generic case and provider keyword arguments as a utility for the most
>> common cases while the patch actually says "it's ok to use kwargs for
>> everything" and if we do that then it should work out of the box.
>>
>> But I almost always use username+password only so I don't know if some
>> parameters need special treatment.
>
> PQconnectdb says: To write an empty value, or a value containing
> spaces, surround it with single quotes, e.g., keyword = 'a value'.
> Single quotes and backslashes within the value must be escaped with a
> backslash, i.e., \' and \\.
>
> I'd rather do this in Python than in C, so at this point the patch is
> basically what I've written yesterday minus the **kwargs pass-through.
> Or, alternatively, a few hundred lines of C with dynamic memory
> allocation, tricky conversion of Python objects to byte strings and a
> couple of off-by-one errors peppered in less tested code paths...

Never said doing it in Python is wrong. In fact anything that isn't
time-critical (type conversions, etc.) at this point is OK in Python.

federico

--
Federico Di Gregorio                                       fog@initd.org
 Io non sono romantica. La candelina sul tavolo mi vede e si spegne.
                                                      -- sisterconfusion

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Thu, Nov 17, 2011 at 11:18 AM, Federico Di Gregorio <fog@dndg.it> wrote:

> Never said doing it in Python is wrong. In fact anything that isn't
> time-critical (type conversions, etc.) at this point is OK in Python.

I was also thinking that having the pair connect()/_connect() is
perfect for regression testing: _connect() can be replaced with a stub
to test the arguments conversion without really connecting.

-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Federico Di Gregorio
Date:
On 17/11/11 12:39, Daniele Varrazzo wrote:
> On Thu, Nov 17, 2011 at 11:18 AM, Federico Di Gregorio <fog@dndg.it> wrote:
>
>> > Never said doing it in Python is wrong. In fact anything that isn't
>> > time-critical (type conversions, etc.) at this point is OK in Python.
> I was also thinking that having the pair connect()/_connect() is
> perfect for regression testing: _connect() can be replaced with a stub
> to test the arguments conversion without really connecting.

Wunderful. But please don't rename the C function. Just "import as", to
avoid breaking API (not that I ever encountered Python code using
_psycopg.so directly but one never knows...)

--
Federico Di Gregorio    <mailto:fog@initd.org> <jid:fog@jabber.linux.it>
DISCLAIMER. If I receive a message from you, you are agreeing that:
 1. I am by definition, "the intended recipient".
 2. All information in the email is mine to do with as I see fit and
 make such financial profit, political mileage, or good joke as it lends
 itself to. In particular, I may quote it on USENET or the WWW.
 3. I may take the contents as representing the views of your company.
 4. This overrides any disclaimer or statement of confidentiality that
 may be included on your message.

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Thu, Nov 17, 2011 at 11:41 AM, Federico Di Gregorio <fog@dndg.it> wrote:
> On 17/11/11 12:39, Daniele Varrazzo wrote:
>> On Thu, Nov 17, 2011 at 11:18 AM, Federico Di Gregorio <fog@dndg.it> wrote:
>>
>>> > Never said doing it in Python is wrong. In fact anything that isn't
>>> > time-critical (type conversions, etc.) at this point is OK in Python.
>> I was also thinking that having the pair connect()/_connect() is
>> perfect for regression testing: _connect() can be replaced with a stub
>> to test the arguments conversion without really connecting.
>
> Wunderful. But please don't rename the C function. Just "import as", to
> avoid breaking API (not that I ever encountered Python code using
> _psycopg.so directly but one never knows...)

I wanted to rename it because I've dropped its support to the keyword
arguments: the interface (which is not an API: if sb is using
_psycopg.so he is doing at his own risk) is broken anyway. And because
we are not going to use the C keyword codepath, I want to drop it
altogether: it is not going to be maintained anymore.

-- Daniele

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Federico Di Gregorio
Date:
On 17/11/11 12:49, Daniele Varrazzo wrote:
> On Thu, Nov 17, 2011 at 11:41 AM, Federico Di Gregorio <fog@dndg.it> wrote:
>> > On 17/11/11 12:39, Daniele Varrazzo wrote:
>>> >> On Thu, Nov 17, 2011 at 11:18 AM, Federico Di Gregorio <fog@dndg.it> wrote:
>>> >>
>>>>> >>> > Never said doing it in Python is wrong. In fact anything that isn't
>>>>> >>> > time-critical (type conversions, etc.) at this point is OK in Python.
>>> >> I was also thinking that having the pair connect()/_connect() is
>>> >> perfect for regression testing: _connect() can be replaced with a stub
>>> >> to test the arguments conversion without really connecting.
>> >
>> > Wunderful. But please don't rename the C function. Just "import as", to
>> > avoid breaking API (not that I ever encountered Python code using
>> > _psycopg.so directly but one never knows...)
> I wanted to rename it because I've dropped its support to the keyword
> arguments: the interface (which is not an API: if sb is using
> _psycopg.so he is doing at his own risk) is broken anyway. And because
> we are not going to use the C keyword codepath, I want to drop it
> altogether: it is not going to be maintained anymore.

Agreed.

--
Federico Di Gregorio                                       fog@initd.org
            Bhoe, bhe, bhe. Sono brutto e cattivo. Brutto lama! -- Cuzco

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Fabian Knittel
Date:
Hi Daniele,

Am 17.11.2011 03:02, schrieb Daniele Varrazzo:
> I'd like to hear from somebody else (mostly Fog, but anybody else)
> before having it merged and released: tests and feedbacks are welcome.

Just a test data point: I've tested your current (github) devel branch
and it works for my use cases, which involves going through sqlalchemy
and using the keywords username, password, host, sslmode and sslrootcert.

So thanks a lot for your nice patches. :)

Cheers
Fabian


Attachment

Re: RFC: Extend psycopg2.connect to accept all valid parameters?

From
Daniele Varrazzo
Date:
On Fri, Nov 18, 2011 at 3:51 PM, Fabian Knittel
<fabian.knittel@avona.com> wrote:

> Just a test data point: I've tested your current (github) devel branch
> and it works for my use cases, which involves going through sqlalchemy
> and using the keywords username, password, host, sslmode and sslrootcert.

Great, thanks for letting us know!

-- Daniele