Additional changes for tests and backwards compatibility (#144)

* Additional changes for tests and backwards compat

- Restore previous import path for run_transaction():

  Prevent existing code from breaking on

    from cockroachdb.sqlalchemy import run_transaction

  and add deprecation warning.

- Add TODO re: passing autocommit= to sessionmaker

- Add README.testing.SQLAlchemy.md

* Bump dialect version to 1.4

Author: gordthompson

README.testing.SQLAlchemy.md

@@ -0,0 +1,32 @@
+## Testing this dialect with SQLAlchemy
+
+"setup.cfg" contains a default SQLAlchemy connection URI that should
+work if you have a local instance of CockroachDB installed:
+
+    [db]
+    default=cockroachdb://root@localhost:26257/defaultdb?disable_cockroachdb_telemetry=True
+
+If you want to test against a remote server (or otherwise need to tweak
+the connection URI) simply create a file named "test.cfg" in the same
+folder as "setup.cfg", copy the ``[db]`` section into it, and adjust the
+``default=`` URI accordingly.
+
+The minimum requirements for testing are:
+
+- SQLAlchemy,
+- pytest, and
+- the psycopg2 DBAPI module.
+
+Install them with 
+
+    pip install sqlalchemy pytest psycopg2-binary
+
+Then, to run a complete test simply invoke
+
+    pytest
+
+at a command prompt in your virtual environment.
+
+For more detailed information see the corresponding SQLAlchemy document
+
+https://github.com/sqlalchemy/sqlalchemy/blob/master/README.unittests.rst

cockroachdb/sqlalchemy/__init__.py

@@ -0,0 +1,9 @@
+from sqlalchemy import util
+from sqlalchemy_cockroachdb import run_transaction  # noqa
+
+util.warn_limited(
+    "Importing ``run_transaction`` from ``cockroachdb.sqlalchemy`` is deprecated since "
+    "version %s of this dialect. "
+    "Please import it from ``sqlalchemy_cockroachdb`` instead.",
+    "1.4",
+)

sqlalchemy_cockroachdb/__init__.py

@@ -1,6 +1,7 @@
 from sqlalchemy.dialects import registry as _registry
+from .transaction import run_transaction  # noqa
 
-__version__ = "1.3.4.dev0"
+__version__ = "1.4.0.dev0"
 
 _registry.register(
     "cockroachdb.psycopg2",

sqlalchemy_cockroachdb/base.py

@@ -153,7 +153,7 @@ def initialize(self, connection):
                 "SELECT crdb_internal.increment_feature_counter"
                 + "('SQLAlchemy {version}')".format(version=version)
             )
-            connection.execute(telemetry_query)
+            connection.execute(text(telemetry_query))
 
     def _get_server_version_info(self, conn):
         # PGDialect expects a postgres server version number here,

sqlalchemy_cockroachdb/transaction.py

@@ -35,6 +35,9 @@ def run_transaction(transactor, callback, max_retries=None, max_backoff=0):
         with transactor.connect() as connection:
             return _txn_retry_loop(connection, callback, max_retries, max_backoff)
     elif isinstance(transactor, sqlalchemy.orm.sessionmaker):
+        # TODO: `autocommit=` is deprecated in SQLA 1.4 (to be removed in 2.0).
+        #       Keep this here for the time being (for compatibility with SQLA 1.3),
+        #       but may not be necessary for 1.4 since _txn_retry_loop uses .begin()
         session = transactor(autocommit=True)
         return _txn_retry_loop(session, callback, max_retries, max_backoff)
     else:

test/test_aaa_run_transaction_session.py

@@ -6,7 +6,7 @@
 from sqlalchemy.types import Integer
 import threading
 
-from sqlalchemy_cockroachdb.transaction import run_transaction
+from sqlalchemy_cockroachdb import run_transaction
 
 """ This file is named "test_aaa_run_transaction_session.py" to ensure that it is run before any
     other tests. Testing under SQLA 1.4 revealed that it ran by itself just fine, but if *any*

test/test_aab_run_transaction_core.py

@@ -4,7 +4,7 @@
 from sqlalchemy.types import Integer
 import threading
 
-from sqlalchemy_cockroachdb.transaction import run_transaction
+from sqlalchemy_cockroachdb import run_transaction
 
 """ This file is named "test_aab_run_transaction_core.py" to keep it close to its more
     temperamental "session" sibling.

test/test_suite.py

@@ -1,24 +1,13 @@
-import re
-
-from sqlalchemy import text
+from sqlalchemy import __version__ as sa_version
 from sqlalchemy.testing.suite import *  # noqa
 from sqlalchemy.testing.suite import ComponentReflectionTest as _ComponentReflectionTest
 from sqlalchemy.testing.suite import ExpandingBoundInTest as _ExpandingBoundInTest
 
 
 class ComponentReflectionTest(_ComponentReflectionTest):
-    @testing.skip("cockroachdb")  # noqa
-    def test_deprecated_get_primary_keys(self):
-        # This is removed in the next sqlalchemy release (1.3.18)
-        pass
-
     def test_get_indexes(self, connection):
-        full_version_string = connection.execute(text("SELECT version()")).scalar()
-        result = re.search(r" (v\d+\.\d+\.\d+) \(", full_version_string)
-        if result:
-            version_string = result.group(1)
-            if version_string >= "v20.2":
-                super().test_get_indexes(connection, None, None)
+        if connection.dialect._is_v202plus and sa_version >= "1.4":
+            super().test_get_indexes(connection, None, None)
 
 
 class ExpandingBoundInTest(_ExpandingBoundInTest):