Add RedshiftDialect_redshift_connector#232
Conversation
|
These changes to the tests seem to be slowing down things quite a bit, I'll take another look on Monday. |
| ): | ||
| pass | ||
| @classmethod | ||
| def dbapi(cls): |
There was a problem hiding this comment.
Can this be defined in Psycopg2RedshiftDialectMixin or does it have to be a classmethod on each concrete dialect? I assume this is some hook in the SQLAlchemy machinery, but I'm struggling to find documentation about a dbapi classmethod. Can you point to background on this approach?
There was a problem hiding this comment.
I think we could move this to Psycopg2RedshiftDialectMixin if we did something like this, using the value of driver defined by the dialect (e.g. defined here for the psycopg2cfifi dialect).
class Psycopg2RedshiftDialectMixin(RedshiftDialectMixin):
@classmethod
def dbapi(cls):
try:
return __import__(self.driver)
except ImportError:
raise ImportError(
'No module named {}.'.format(self.driver)
)For the dbapi classmethod, looking to the DefaultDialect class (from which PGDialect inherits from) it is used in DefaultDialect.connect(), as well as for defining the dbapi paramstyle that will be used.
I was unable to find any docs for the dpapi classmethod either, as I stumbled upon it when looking for where the driver's connect method is called.
jklukas
left a comment
There was a problem hiding this comment.
This looks good. I've kicked off integration tests. Do you have any more info on duration of tests with this change?
| @classmethod | ||
| def dbapi(cls): | ||
| try: | ||
| return __import__(cls.driver) |
There was a problem hiding this comment.
It looks like https://docs.python.org/3/library/importlib.html#importlib.import_module is preferred over __import__.
It seems like the tests which make a connection to a Redshift cluster take about twice as long. The unit tests are pretty quick. I think this has something to do with the dynamic parameterization of the fixtures, as the slow down came once I introduced that logic. |
jklukas
left a comment
There was a problem hiding this comment.
LGTM. Kicked off integration tests again and will merge when done.
|
thanks for the quick turn around, @jklukas! When do you plan on making the next release? We'd like to get this feature out to customers as soon as we can. |
|
I just pushed 0.8.6 which includes this new connector. |
Adds a dialect to support use of redshift_connector database driver.
Modifies sqlalchemy-redshift init requirement for psycopg2 to dialect
dbapimethod to avoid raising an ImportError for users using RedshiftDialect_redshift_connectorModifies pytest to accept a command line option,
dbdriver, to provide 1 or more database drivers to run the test suite against (e.g.—dbdriver psycopg2 —dbdriver redshift_connector). By default tests will run forpsycopg2andpsycopg2cffiModifies tox configuration to test with
—dbdriver redshift_connectorfor compatible Python versions (i.e. py36 >=), and to use a version ofrequests.Reworks
test_dialects.pyto use a mock sqlalchemy engine.Todos