trinodb / trino-python-client

Python client for Trino
Apache License 2.0
309 stars 151 forks source link

SQLAlchemy 2.0 Compatibility #307

Closed mathiasritter closed 1 year ago

mathiasritter commented 1 year ago

Description

In the do_execute function of SQLAlchemy dialect, an if statement was accessing context.should_autocommit. This property has been removed in SQLAlchemy 2.0.

To fix, the if statement can simply be removed as it is no longer needed. See #291 for more details.

Resolves #291

Non-technical explanation

The current dialect is incompatible with SQLAlchemy version 2.0. This change makes the dialect compatible.

Release notes

( ) This is not user-visible or docs only and no release notes are required. ( ) Release notes are required, please propose a release note for me. (x) Release notes are required, with the following suggested text:

* Add support for SQLAlchemy 2.0. ({issue}`291`)
cla-bot[bot] commented 1 year ago

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: mritter31. This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails
mathiasritter commented 1 year ago

Thanks @mdesmet, that was what I was looking for.

Unfortunately, the tests for 2.0 are failing because of an issue in sqlalchemy-utils library. That issue has been fixed since October, but they haven't released a new version since July... See: https://github.com/kvesteri/sqlalchemy-utils/issues/666

hashhar commented 1 year ago

We're using that library only in tests if I remember correctly - can we remove that dependency with some alternative code? It's used in two tests - just to verify whether sqla.inspect(engine).get_table_names(schema) returns only tables and whether sqla.inspect(engine).get_view_names(schema) only returns views.

Would those tests still work if we directly executed CREATE VIEW on the underlying connection instead of going via SQLA?

mathiasritter commented 1 year ago

@hashhar They have just released a new version of sqlalchemy-utils that fixed the issue 😄 got some other issues now though, that I will look into later.

mdesmet commented 1 year ago

@hashhar : PTAL

cla-bot[bot] commented 1 year ago

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Mathias Ritter. This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails
hashhar commented 1 year ago

LGTM. @mathiasritter I assume you've sent the CLA already (and it matches the email you use in your commits)? If not please do. If sent already I'll take care of it.

mathiasritter commented 1 year ago

@hashhar Yes I’ve already sent it