search

exasol / sqlalchemy-exasol

SQLAlchemy dialect for EXASOL
https://exasol.github.io/sqlalchemy-exasol/
Other
34 stars 28 forks source link

Support Sqlalchemy 1.4 #106

Closed naresh-kumar-by closed 1 year ago

naresh-kumar-by commented 3 years ago

sqlalchemy_exasol python package is not compatible with new version of sqlalchemy 1.4+ when used with turbodbc dialect. Below is the code to reproduce this issue.


from sqlalchemy import create_engine

exa_engine = create_engine("exa+turbodbc://sys:exasol@exasoldb:8888")
exa_engine.connect()

################################### ERROR #########################################

    exec(code, run_globals)
  File "/app/test.py", line 6, in <module>
    exa_engine.connect()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 3166, in connect
    return self._connection_cls(self, close_with_result=close_with_result)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 96, in __init__
    else engine.raw_connection()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 3245, in raw_connection
    return self._wrap_pool_connect(self.pool.connect, _connection)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 3212, in _wrap_pool_connect
    return fn()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 301, in connect
    return _ConnectionFairy._checkout(self)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 761, in _checkout
    fairy = _ConnectionRecord.checkout(pool)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 419, in checkout
    rec = pool._do_get()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/impl.py", line 145, in _do_get
    self._dec_overflow()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/util/langhelpers.py", line 70, in __exit__
    compat.raise_(
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/util/compat.py", line 207, in raise_
    raise exception
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/impl.py", line 142, in _do_get
    return self._create_connection()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 247, in _create_connection
    return _ConnectionRecord(self)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 362, in __init__
    self.__connect()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 616, in __connect
    pool.dispatch.connect.for_modify(
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 329, in _exec_w_sync_on_first_run
    self(*args, **kw)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 343, in __call__
    fn(*args, **kw)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/util/langhelpers.py", line 1676, in go
    return once_fn(*arg, **kw)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/create.py", line 675, in first_connect
    dialect.initialize(c)
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 394, in initialize
    self.server_version_info = self._get_server_version_info(
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy_exasol/turbodbc.py", line 82, in _get_server_version_info
    result = connection.execute(query).fetchone()[0].split('.')
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1247, in execute
    return self._exec_driver_sql(
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1546, in _exec_driver_sql
    ret = self._execute_context(
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1696, in _execute_context
    context.pre_exec()
  File "/root/venv/lib/python3.8/site-packages/sqlalchemy_exasol/base.py", line 336, in pre_exec
    if self.isdelete or self.root_connection.dialect.server_version_info < (4, 1, 8):
TypeError: '<' not supported between instances of 'NoneType' and 'tuple'
naresh-kumar-by commented 3 years ago

@tkilias at BlueYonder (formerly known as JDA), we are working on a fix and adding support for new sqlalchemy version, we might require additional help.

jank commented 3 years ago

Consider dropping support for Python 2.7 with a new release that supports SQLAlchemy 1.4. This would fix https://github.com/blue-yonder/sqlalchemy_exasol/issues/93.

alxdb commented 3 years ago

I'm also encountering this issue

vasu-sharma-by commented 2 years ago

Has there been any development on this? I'm also facing this issue.

Nicoretti commented 2 years ago

Started the migration to SQLA 1.4.

For more details see also PR #191

Nicoretti commented 2 years ago

Status Update

After investigating various aspects of the failing custom merge statement, we concluded that a more thorough rewrite, also considering the "new" api concept of SQLA, is the right way to address this issue. Still beforehand we want to make sure the general functionality of the plugin is working with SQLA version 1.4. Therefore first we will focus on addressing the more general issues caused by the SQLA upgrade.

Status Overview

Failing Test(s)

So far 4 major categories of failing tests have been identified:

Attachments

Nicoretti commented 2 years ago

Status Update

While fixing individual tests it became apparent that the majority of the not working tests is rather crashing than failing. Due to the fact that a lot of the crashing tests looked like they have a similar and non api change related cause, I started investigating various of this crashes.

What I have found so far?

Assumptions

Test(s) Status

pyodbc

63 :no_entry_sign: failed, 251 :heavy_check_mark: passed , 321 :yellow_circle: skipped, 144 :boom: errors

turbodbc

76 :no_entry_sign: failed, 255 :heavy_check_mark: passed , 335 :yellow_circle: skipped, 120 :boom: errors

:warning: Finding and fixing the cause(s) of the crashing tests will be the main focus, before continuing with the failing tests :warning:

Nicoretti commented 2 years ago

Status Update

Further investigation's have shown that the basic SQLA test suite mostly is intact after the upgrade to 1.4 (14 failures), when run in "isolation". Various exasol specific test suites:

seem to have negative side effects which cause 100+ tests to :boom: fail/crash, if run after those test suites. This further strengthens the case for the assumptions mentioned in the previous update:

Also, this narrows down the potential root cause(s).

Remark(s)

Common to all those test suites to be that they add/remove schemas. For test/test_regression.py it have been proven that the schema manipulating test (TranslateMap) causes some negative side effect on following test suits.

Notes & Ideas (from discussion with @tkilias )

Next Steps

Nicoretti commented 1 year ago

Status Update

Further analysis has shown that the majority of the crashing tests was caused, due to implicit schema changes which have been a "side effect" of integration tests added by exasol.

In order to address this, the test setup/execution have been adjusted. Also further improvements regarding test robustness have been identified and are track in #209.

Test(s) Status

sqlalchemy

exasol

Next Steps

Nicoretti commented 1 year ago

Status Update

A first set of changes which addresses various issues to support SQLA 1.4 have been merged into the the development branch (dev-sqla-1.4).

A first superficial analysis of the remaining failing and crashing tests has shown the following:

  1. The remaining crashing tests seem to stem from a behavioral change of sqla, so that the pyodbc based dialect now uses an additional connection for the reflection. This causes tests using the AssertionPool to crash.

  2. There are no obvious low hanging fruit's to fix

  3. A couple of the failing tests show signs that indicate, SQLA has changed how it is using/interacting with the DDL compiler of the dialect(s). Such indications are:

    • Missing/Uncreated Tables
    • Unexpected Table names

Test(s) Status

sqlalchemy

exasol

Next Steps

Nicoretti commented 1 year ago

Status Update

Test(s) Status

sqlalchemy

exasol

Nicoretti commented 1 year ago

Status Update

Test(s) Status

sqlalchemy

exasol

Next Steps

Nicoretti commented 1 year ago

Status Update

Test(s) Status

sqlalchemy

exasol

Next Steps

Related Issues

Nicoretti commented 1 year ago

Status Update

Test(s) Status

sqlalchemy

exasol

Next Steps

Related Issues

Nicoretti commented 1 year ago

:information_source:

It have been decided by PM, that the support for the custom merge statement, is dropped.
If we will be notified, that someone depends on this feature, we are open to add it again.
jcharlytown commented 1 year ago

Hi @Nicoretti , we are using sqlalchemy-exasol at my current company. We still need to evaluate if we're using custom merge statements within our code base. What's the ETA for giving feedback on this?

Also, one more question: I see that the 1.4 branch drops support for <1.4. Will a version which supports <1.4 continue to be maintained, or will this project stop supporting <1.4 altogether once this effort completes?

Thanks!

Nicoretti commented 1 year ago

Hi @jcharlytown,

We still need to evaluate if we're using custom merge statements within our code base. What's the ETA for giving feedback on this?

Take your time, the changes will be published as major release, if your dependency is pinned to stay on a specific major version you will be good.

:spiral_notepad: Side note: If the Merge statement is added again it's very likely that we need to change the API (usage/syntaxt) of it.

Regarding your second question

Also, one more question: I see that the 1.4 branch drops support for <1.4. Will a version which supports <1.4 continue to be maintained, or will this project stop supporting <1.4 altogether once this effort completes?

In order to answer this in more detail, I will need to consult our project management about it. (I'll try to come back to you with an answer, as soon as possible)

best Nico

Nicoretti commented 1 year ago

Hi @jcharlytown,

I had a brief discussion with our project management, and we concluded not to maintain two lines.

Why is that?

best Nico

See also https://github.com/exasol/sqlalchemy-exasol/discussions/250