Make keyword arguments in Python DB API connect() function more standard #2901
Labels
Milestone
Comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Date: 2011-10-12 13:10:34 +0200
From: Evert Rol <<evert.astro>>
To: clients devs <>
Version: 11.5.3 (Aug2011-SP1) [obsolete]
CC: @drstmane
Last updated: 2011-10-26 13:22:01 +0200
Comment 16395
Date: 2011-10-12 13:10:34 +0200
From: Evert Rol <<evert.astro>>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_1) AppleWebKit/535.1 (KHTML, like Gecko) Chrome/14.0.835.202 Safari/535.1
Build Identifier:
While not specified, most Python DB APIs use the same keywords in the connect() function. These are also suggested in footnote 1 of the Python DB API specification (PEP 249):
dsn Data source name as string
user User name as string (optional)
password Password as string (optional)
host Hostname (optional)
database Database name (optional)
Currently, the monetdb DB API uses username instead of user, and hostname instead of host.
Use of the 'user' and 'host' keywords will make the DB API more compatible with existing Python DB APIs and the suggested guideline.
Reproducible: Always
I guess it would be trivial to have both user and username, and both host and hostname allowed, for backwards compatibility.
Comment 16396
Date: 2011-10-12 13:13:19 +0200
From: @grobian
yup, agree on that. Would you mind creating a patch?
Comment 16397
Date: 2011-10-12 14:37:26 +0200
From: Evert Rol <<evert.astro>>
Created attachment 79
Suggested patch
Attached patch works for me, but has not (yet) been thoroughly tested.
(NB: I don't have access to the repo, so filenames at top of patch are incorrect; in case of blindly applying this patch ;-)
Comment 16398
Date: 2011-10-12 14:42:01 +0200
From: @grobian
LGTM.
Do you intend to test it a bit further?
Comment 16399
Date: 2011-10-12 17:59:24 +0200
From: Evert Rol <<evert.astro>>
Not really.
It's used in a practical use case, which serves as the most important test for me.
I did test for the various variants of user/username and host/hostname (including both, to test the precedence); if wanted, I can include that test in the unit test (I think that's a single Python file, from what I could find).
Other than that, I felt the change was trivial enough to not overly bother with it (famous last words).
Comment 16400
Date: 2011-10-12 18:25:29 +0200
From: @grobian
I concur, but your comment 2 suggested you'd know how to test it better. Consider it applied.
Comment 16401
Date: 2011-10-12 18:32:39 +0200
From: @grobian
bah, forgot bugref in commit:
http://dev.monetdb.org/hg/MonetDB/rev/2dd92bb63b34
Comment 16419
Date: 2011-10-14 09:22:49 +0200
From: @drstmane
Would it be possible to also add a test for this one?
Comment 16422
Date: 2011-10-14 10:14:11 +0200
From: @drstmane
re-opened to remind us that we should consider adding a test
Comment 16439
Date: 2011-10-17 14:54:27 +0200
From: Evert Rol <<evert.astro>>
Created attachment 81
Unit test patch
Not the most pretty test, but this should test various combinations of user/username and host/hostname.
I've assumed runtests.py serves as the unit test for the Python client.
Comment 16446
Date: 2011-10-19 17:19:05 +0200
From: @sjoerdmullender
Changeset 1045c895ec9e made by Sjoerd Mullender sjoerd@acm.org in the MonetDB repo, refers to this bug.
For complete details, see http//devmonetdborg/hg/MonetDB?cmd=changeset;node=1045c895ec9e
Changeset description:
Comment 16447
Date: 2011-10-19 17:20:18 +0200
From: @sjoerdmullender
I added a test.
I didn't use Evert's test since we don't actually use those unit tests.
The text was updated successfully, but these errors were encountered: