support both sqlalchemy engines and connections #10105

graingert · 2015-05-11T15:35:25Z

artemyk · 2015-05-12T05:39:08Z

This looks good and would also resolve #7877 I believe.
I think some tests will probably be requested.

jorisvandenbossche · 2015-05-12T08:39:25Z

@graingert Thanks for working on this!

Certainly look also to the discussion in #7877. At the end there is link to a patch of @tr11 which does something similar.

Some quick thoughts:

indeed, as @artemyk mentions, this needs some tests.
The current tests are failing with both "ImportError: cannot import name interfaces" (I think using ``sqlalchemy.engine.Connectable` should work for all versions), and with an error in SQLAlchemyTransaction itself. You can see the output on the travis log (click the red cross above next to the commit to see the logs)
I think also this will not yet work for to_sql and read_sql_table? (how you implemented it now, but is should work for those as well)

jorisvandenbossche · 2015-05-20T19:35:08Z

@graingert Some of the tests keep on running. I already restarted them a few times, so it seems this is caused by this PR. Did you try to run the tests locally? (only the sql tests is enough)

graingert · 2015-05-27T16:13:58Z

pandas/io/tests/test_sql.py

@@ -2180,6 +2237,7 @@ def setUp(self):
        except:
            pass
        else:
+            self.addCleanup(self._try_close_db)


For the tests to pass on my machine I need to close the database connection after each test.

graingert · 2015-05-27T16:17:38Z

@jorisvandenbossche I've debugged all the tests.

Once the travis builds pass I'll remove the commits Fix TestXMySQL and increase verbosity so I can see what's going wrong

graingert · 2015-05-27T16:18:39Z

I think Fix TestXMySQL should probably go in another PR

graingert · 2015-05-28T21:38:04Z

@jorisvandenbossche @artemyk I can't replicate the issues in 4b2f087...1037144 locally do you have any ideas?

graingert · 2015-05-28T23:18:32Z

@artemyk @jorisvandenbossche I'm pretty sure I've got it now.

I'll clean up this PR with a repo tomorrow

graingert · 2015-05-28T23:24:49Z

When running with two queries at the same time, if one raises a warning and the other doesn't PyMySQL raises a TypeError.

        proc = """DROP PROCEDURE IF EXISTS get_testdb;
         CREATE PROCEDURE get_testdb ()

         BEGIN
             SELECT * FROM test_procedure;
         END"""

The DROP PROCEDURE was raising a warning on Travis because the get_testdb procedure was missing, however on my local machine I was not destroying/rebuilding the database each test run which meant I still had the "get_testdb" procedure.

artemyk · 2015-05-29T06:00:21Z

@graingert Glad you've figured it out. I'm not sure execute necessarily supports multiple statements, for example some DBAPIs (including PyMySQL if I'm not mistaken) have an executemany method for this purpose.

jreback · 2015-06-04T12:18:13Z

pls rebase on master, some issues have been fixed with the builds

graingert · 2015-06-06T19:23:54Z

@jorisvandenbossche done @jreback yep, rebase fixed those issues thanks.

graingert · 2015-06-10T16:15:35Z

@jorisvandenbossche @jreback casual nudge

graingert · 2015-06-25T13:15:06Z

@jorisvandenbossche @jreback hey guys, I really need this feature in production... can you guys merge this if you think it's useful

jreback · 2015-06-25T13:20:42Z

@graingert lgtm, but @jorisvandenbossche may have some more comments.

if you are actually running off of master (in production)?, then you can just merge this into your local master. This would be in 0.17.0 which won't be out for prob 2 months.

jorisvandenbossche · 2015-06-25T15:02:45Z

Sorry for the slow response. But I will look at it this evening, I promise!

Short comment: ideally, I would like to see a test that now fails (so a test that is only run for the test class that uses a connection, and not an engine), to be sure that with a later refactor this ability is not lost again.

graingert · 2015-06-25T15:07:11Z

@jorisvandenbossche that test is added here: https://github.com/pydata/pandas/pull/10105/files#diff-052a926a9b2c33a84e27d51c8fad3466R1372

jorisvandenbossche · 2015-06-25T15:16:32Z

Yes, I saw that, but if I understand correctly, this test is ran for both the engine tests as the connection tests?
I was thinking that this enhancement adds something that was not possible now with engines (a use case for which you need the connection, and can't use the engine)? Eg using a temporary table. So it would be useful to have a test with such a use case, so we ensure that such functionality keeps working.

jorisvandenbossche · 2015-06-25T15:22:30Z

pandas/io/sql.py

@@ -139,7 +139,7 @@ def execute(sql, con, cur=None, params=None):
    ----------
    sql : string
        Query to be executed
-    con : SQLAlchemy engine or sqlite3 DBAPI2 connection
+    con : SQLAlchemy connectable or sqlite3 DBAPI2 connection


I worry a bit that the term 'connectable' is not really known to most users? Maybe just use engine/connection?

graingert · 2015-06-25T15:41:07Z

Ah I see what you mean

jorisvandenbossche · 2015-07-03T13:19:41Z

pandas/io/tests/test_sql.py

+        self.__tx.rollback()
+        self.conn.close()
+        self.conn = self.__engine
+        self.pandasSQL = sql.SQLDatabase(self.__engine)


Does this line have to be here?

not currently, it's just so this class cleans up everything back to how it was from the superclass's setup.

This is for principal of least surprise for those superclasses.

OK, but before each test is run, the setup will overwrite this anyway again?

this is so that when the tearDown method gets called in the superclasses the instance is exactly how that superclass set it up in setUp

ah, yes, OK

jorisvandenbossche · 2015-07-03T13:28:45Z

Thanks for adding the temp table test! Gave it a last round, see last comments.

Last question: can you squash the commits? Then I will merge when it is green again

update pymysql to 0.6.3 to avoid cursor bugs Add documentation and tests for SQLAlchemy connectables explicit reference to connection/engine in docs Temporary table test pass compile the connectable

graingert · 2015-07-03T13:40:24Z

fixed, rebased, squashed It's party time: 🍰 ✨

jorisvandenbossche · 2015-07-03T17:44:53Z

@graingert Thanks a lot! (for the contribution, and also for the patience ..)

support both sqlalchemy engines and connections

graingert · 2015-07-03T20:44:15Z

Woot, thanks very much!

graingert force-pushed the sqlalchemy-connectable branch from d4a42a6 to 6e80100 Compare May 11, 2015 15:35

graingert changed the title ~~support both sqlalchemy engines and connections Fixes #10104~~ support both sqlalchemy engines and connections May 11, 2015

jorisvandenbossche mentioned this pull request May 12, 2015

ENH: support both SQLAlchemy engines and connections #7877

Closed

graingert force-pushed the sqlalchemy-connectable branch 4 times, most recently from ef122d6 to 5c7d352 Compare May 13, 2015 09:10

jorisvandenbossche added the IO SQL to_sql, read_sql, read_sql_query label May 20, 2015

jorisvandenbossche added this to the Next Major Release milestone May 20, 2015

graingert force-pushed the sqlalchemy-connectable branch 5 times, most recently from 5fd3d9a to 9c1a737 Compare May 27, 2015 16:11

graingert reviewed May 27, 2015
View reviewed changes

graingert force-pushed the sqlalchemy-connectable branch 2 times, most recently from 40e09e0 to 71fd12e Compare May 27, 2015 16:15

graingert force-pushed the sqlalchemy-connectable branch from 40e09e0 to 56094aa Compare May 28, 2015 08:57

graingert mentioned this pull request May 28, 2015

TypeError for multiple queries when one raises a warning PyMySQL/PyMySQL#343

Closed

graingert force-pushed the sqlalchemy-connectable branch 2 times, most recently from 74e0a11 to c34fd2b Compare May 29, 2015 09:10

graingert force-pushed the sqlalchemy-connectable branch 4 times, most recently from e15f56a to 66283c3 Compare June 6, 2015 19:23

graingert force-pushed the sqlalchemy-connectable branch from 66283c3 to 9d72dd9 Compare June 25, 2015 15:07

jorisvandenbossche reviewed Jun 25, 2015
View reviewed changes

graingert force-pushed the sqlalchemy-connectable branch 2 times, most recently from 5ec7a24 to ec5c9d9 Compare June 26, 2015 11:13

jorisvandenbossche reviewed Jul 3, 2015
View reviewed changes

support both sqlalchemy engines and connections Fixes pandas-dev#7877

7327f6b

update pymysql to 0.6.3 to avoid cursor bugs Add documentation and tests for SQLAlchemy connectables explicit reference to connection/engine in docs Temporary table test pass compile the connectable

graingert force-pushed the sqlalchemy-connectable branch from a9583bc to 7327f6b Compare July 3, 2015 13:39

jorisvandenbossche added a commit that referenced this pull request Jul 3, 2015

Merge pull request #10105 from graingert/sqlalchemy-connectable

80597dc

support both sqlalchemy engines and connections

jorisvandenbossche merged commit 80597dc into pandas-dev:master Jul 3, 2015

graingert deleted the sqlalchemy-connectable branch July 3, 2015 19:23

jorisvandenbossche mentioned this pull request Jul 19, 2015

support sql transactions #10617

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support both sqlalchemy engines and connections #10105

support both sqlalchemy engines and connections #10105

graingert commented May 11, 2015

artemyk commented May 12, 2015

jorisvandenbossche commented May 12, 2015

jorisvandenbossche commented May 20, 2015

graingert May 27, 2015

graingert commented May 27, 2015

graingert commented May 27, 2015

graingert commented May 28, 2015

graingert commented May 28, 2015

graingert commented May 28, 2015

artemyk commented May 29, 2015

jreback commented Jun 4, 2015

graingert commented Jun 6, 2015

graingert commented Jun 10, 2015

graingert commented Jun 25, 2015

jreback commented Jun 25, 2015

jorisvandenbossche commented Jun 25, 2015

graingert commented Jun 25, 2015

jorisvandenbossche commented Jun 25, 2015

jorisvandenbossche Jun 25, 2015

graingert commented Jun 25, 2015

jorisvandenbossche Jul 3, 2015

graingert Jul 3, 2015

jorisvandenbossche Jul 3, 2015

graingert Jul 3, 2015

jorisvandenbossche Jul 3, 2015

jorisvandenbossche commented Jul 3, 2015

graingert commented Jul 3, 2015

jorisvandenbossche commented Jul 3, 2015

graingert commented Jul 3, 2015

support both sqlalchemy engines and connections #10105

support both sqlalchemy engines and connections #10105

Conversation

graingert commented May 11, 2015

artemyk commented May 12, 2015

jorisvandenbossche commented May 12, 2015

jorisvandenbossche commented May 20, 2015

graingert May 27, 2015

Choose a reason for hiding this comment

graingert commented May 27, 2015

graingert commented May 27, 2015

graingert commented May 28, 2015

graingert commented May 28, 2015

graingert commented May 28, 2015

artemyk commented May 29, 2015

jreback commented Jun 4, 2015

graingert commented Jun 6, 2015

graingert commented Jun 10, 2015

graingert commented Jun 25, 2015

jreback commented Jun 25, 2015

jorisvandenbossche commented Jun 25, 2015

graingert commented Jun 25, 2015

jorisvandenbossche commented Jun 25, 2015

jorisvandenbossche Jun 25, 2015

Choose a reason for hiding this comment

graingert commented Jun 25, 2015

jorisvandenbossche Jul 3, 2015

Choose a reason for hiding this comment

graingert Jul 3, 2015

Choose a reason for hiding this comment

jorisvandenbossche Jul 3, 2015

Choose a reason for hiding this comment

graingert Jul 3, 2015

Choose a reason for hiding this comment

jorisvandenbossche Jul 3, 2015

Choose a reason for hiding this comment

jorisvandenbossche commented Jul 3, 2015

graingert commented Jul 3, 2015

jorisvandenbossche commented Jul 3, 2015

graingert commented Jul 3, 2015