SQLAlchemy support #81

dungdm93 · 2021-04-02T08:22:25Z

findepi · 2021-04-21T07:46:47Z

Let me know when this becomes non-draft.

dungdm93 · 2021-04-21T12:58:15Z

@findepi Do you know why CI say ModuleNotFoundError: No module named 'trino.sqlalchemy'?
I run test in my local machine, it works fine.

hashhar · 2021-04-21T13:26:50Z

@dungdm93 Could it be due to https://github.com/trinodb/trino-python-client/blob/16feda14efb1a0985053d92338313f32548d1095/trino/__init__.py (no trino.sqlalchemy entry there).

cc: @joshthoward

trino/__init__.py

findepi · 2021-04-26T06:59:04Z

feat: support python 3.5

this shouldn't be needed. i just merged #83 so that you can drop this one

hashhar

Sorry for the delay in reviewing. Thanks a lot for your work.

A few comments.

I did not review feat: TrinoTypeCompiper datatypes yet.

Also, I did not completely understand datatype.py added in 66f6666.

trino/sqlalchemy/compiler.py

trino/sqlalchemy/dialect.py

hashhar · 2021-05-03T06:42:34Z

trino/sqlalchemy/dialect.py

+        else:
+            raise ValueError(f'Unexpected database format {url.database}')
+
+        username = kwargs.pop('username', 'anonymous')


Why provide a default username? The default for all other clients is an empty/missing username.
Also you can inline this into the line below.

If username is empty, we get this error:

trino.exceptions.HttpError: error 401: b'Basic authentication or X-Trino-User must be sent'

@findepi @hashhar Do you think we should use OS username by default in Python client as well?

@ebyhr is that implemented any where else (like CLI or JDBC)?

@ebyhr @dungdm93 yes, we should default to os username, as CLI does, or require username to be set.
JDBC requires user to be set: https://github.com/trinodb/trino/blob/ceb2d1b8238c4b6949abf2e7f506aab83bf1ea02/client/trino-jdbc/src/main/java/io/trino/jdbc/ConnectionProperties.java#L157

@findepi @ebyhr @hashhar I think we should going to the same direction as JDBC does.

So now user is required.
This must be specify either in URL or in connect_args

engine = create_engine("trino://username@localhost:8080/mysql") # or engine = create_engine( "trino://localhost:8080/mysql", connect_args=dict(user="username") )

trino/sqlalchemy/dialect.py

trino/__init__.py

integration_tests/test_dbapi.py

.github/workflows/ci.yml

trino/sqlalchemy/compiler.py

trino/sqlalchemy/dialect.py

hashhar · 2021-05-30T12:37:28Z

@dungdm93 Did you get a chance to address the comments? I would really love to see this merged. Let me know if there's anything I can help with to push this over the finish line.

dungdm93 · 2021-06-02T00:12:08Z

Sorry @hashhar to let you waiting. I so busy recently. I’ll try to finish this task next week.

buremba · 2021-08-27T11:51:46Z

@dungdm93 Did you have time to add the comments? I can help and create another PR from this one if you're busy, please let me know in that case.

hashhar

3rd commit: "test: dialect.create_connect_args"

tests/sqlalchemy/test_dialect.py

trino/auth.py

hashhar

4th commit: "refactor: change IsolationLevel to Enum"

Can we extract the formatting related changes to a separate first commit? It makes it much easier to see the places where actual changes have happenned.

A few comments. I'll do an in-depth review once again after reading up about the sqlalchemy api expectations.

tests/sqlalchemy/test_dialect.py

trino/transaction.py

esemeniuc · 2021-10-13T17:03:27Z

How is the current state of the port? Is it mostly usable?

hashhar

Sorry for the long delay @dungdm93.

I think it's almost ready to be merged % comments.

cc: @aalbu

tests/sqlalchemy/conftest.py

tests/sqlalchemy/test_datatype_parse.py

trino/__init__.py

trino/dbapi.py

trino/sqlalchemy/compiler.py

trino/sqlalchemy/datatype.py

trino/sqlalchemy/dialect.py

hashhar

Looks mostly good to me. Couple of comments.

@findepi Do you want to take a look?

tests/sqlalchemy/test_datatype_parse.py

tests/sqlalchemy/test_dialect.py

trino/__init__.py

trino/sqlalchemy/datatype.py

trino/sqlalchemy/dialect.py

hashhar · 2021-11-25T06:04:30Z

Also, I can't see any way to logically keep the commits separate - please squash them into a single one.

Signed-off-by: Đặng Minh Dũng <dungdm93@live.com>

dungdm93 · 2021-11-25T08:43:44Z

@hashhar updated as your comments.
I also squash all commits and has some feedback.

hashhar · 2021-12-02T22:45:58Z

Merged as da1441f (and preceding commits). There were some conflicts that I resolved (and split into commits). Your authorship is retained on all commits.

Thanks so much for continuing to work on this @dungdm93 and getting it merged. ❤️ This will be available in the next release which should follow soon.

cla-bot bot added the cla-signed label Apr 2, 2021

dungdm93 force-pushed the sqlalchemy branch 3 times, most recently from 80b334f to f44da94 Compare April 3, 2021 11:43

dungdm93 force-pushed the sqlalchemy branch from f44da94 to 75df687 Compare April 12, 2021 10:22

dungdm93 force-pushed the sqlalchemy branch from 75df687 to 16feda1 Compare April 20, 2021 04:13

hashhar reviewed Apr 21, 2021

View reviewed changes

trino/__init__.py Outdated Show resolved Hide resolved

dungdm93 mentioned this pull request Apr 21, 2021

feat: first step native support Trino apache/superset#13105

Merged

6 tasks

dungdm93 force-pushed the sqlalchemy branch from 3332e93 to 8f68421 Compare April 26, 2021 04:28

dungdm93 marked this pull request as ready for review April 26, 2021 04:30

dungdm93 force-pushed the sqlalchemy branch 2 times, most recently from bfdfc05 to f448713 Compare April 26, 2021 08:39

findepi requested a review from hashhar April 26, 2021 11:48

hashhar reviewed May 3, 2021

View reviewed changes

buremba mentioned this pull request Aug 27, 2021

trino-python-client returns dtype string for datetime/timestamp fields #32

Closed

dungdm93 force-pushed the sqlalchemy branch 7 times, most recently from ff00675 to 0ad3563 Compare September 2, 2021 09:53

dungdm93 requested a review from hashhar September 2, 2021 11:42

hashhar reviewed Sep 3, 2021

View reviewed changes

tests/sqlalchemy/test_dialect.py Show resolved Hide resolved

tests/sqlalchemy/test_dialect.py Show resolved Hide resolved

trino/auth.py Show resolved Hide resolved

hashhar reviewed Sep 3, 2021

View reviewed changes

tests/sqlalchemy/test_dialect.py Outdated Show resolved Hide resolved

trino/transaction.py Outdated Show resolved Hide resolved

trino/transaction.py Show resolved Hide resolved

dungdm93 force-pushed the sqlalchemy branch 4 times, most recently from a61c292 to 8bb13b7 Compare September 5, 2021 03:49

dungdm93 force-pushed the sqlalchemy branch 2 times, most recently from beff693 to 60c27d1 Compare September 12, 2021 15:42

This was referenced Sep 13, 2021

SQLAlchemy dialect support more Authentication methods #106

Closed

SQLAlchemy dialect support more SQL datatypes #107

Open

dungdm93 force-pushed the sqlalchemy branch 2 times, most recently from 9c7c8cd to 0ad84e8 Compare September 13, 2021 11:55

dungdm93 requested a review from hashhar September 13, 2021 12:00

dungdm93 force-pushed the sqlalchemy branch 2 times, most recently from 6e09a05 to 5ff9a27 Compare September 18, 2021 10:08

dungdm93 force-pushed the sqlalchemy branch from 5ff9a27 to 0958642 Compare September 23, 2021 10:43

hashhar reviewed Oct 20, 2021

View reviewed changes

hovaesco reviewed Nov 19, 2021

View reviewed changes

trino/sqlalchemy/dialect.py Outdated Show resolved Hide resolved

dungdm93 force-pushed the sqlalchemy branch from 0958642 to 53e1c67 Compare November 24, 2021 14:20

hashhar reviewed Nov 25, 2021

View reviewed changes

feat: support sqlalchemy

c5d19b5

Signed-off-by: Đặng Minh Dũng <dungdm93@live.com>

dungdm93 force-pushed the sqlalchemy branch from bd18bf3 to c5d19b5 Compare November 25, 2021 08:36

hashhar closed this Dec 2, 2021

hashhar mentioned this pull request Dec 15, 2021

SQLAlchemy compatibility #13

Closed

This was referenced May 5, 2022

Deprecated sqlalchemy-trino in favor of trino-python-client dungdm93/sqlalchemy-trino#48

Merged

feat: switch from sqlalchemy-trino to trino-python-client apache/superset#19957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQLAlchemy support #81

SQLAlchemy support #81

dungdm93 commented Apr 2, 2021 •

edited

Loading

findepi commented Apr 21, 2021

dungdm93 commented Apr 21, 2021

hashhar commented Apr 21, 2021

findepi commented Apr 26, 2021

hashhar left a comment

hashhar May 3, 2021

dungdm93 Sep 2, 2021

ebyhr Sep 3, 2021 •

edited

Loading

dungdm93 Sep 3, 2021

findepi Sep 3, 2021

dungdm93 Sep 12, 2021 •

edited

Loading

dungdm93 Sep 13, 2021

hashhar commented May 30, 2021

dungdm93 commented Jun 2, 2021

buremba commented Aug 27, 2021 •

edited

Loading

hashhar left a comment

hashhar left a comment

esemeniuc commented Oct 13, 2021

hashhar left a comment

hashhar left a comment

hashhar commented Nov 25, 2021

dungdm93 commented Nov 25, 2021

hashhar commented Dec 2, 2021

SQLAlchemy support #81

SQLAlchemy support #81

Conversation

dungdm93 commented Apr 2, 2021 • edited Loading

findepi commented Apr 21, 2021

dungdm93 commented Apr 21, 2021

hashhar commented Apr 21, 2021

findepi commented Apr 26, 2021

hashhar left a comment

Choose a reason for hiding this comment

hashhar May 3, 2021

Choose a reason for hiding this comment

dungdm93 Sep 2, 2021

Choose a reason for hiding this comment

ebyhr Sep 3, 2021 • edited Loading

Choose a reason for hiding this comment

dungdm93 Sep 3, 2021

Choose a reason for hiding this comment

findepi Sep 3, 2021

Choose a reason for hiding this comment

dungdm93 Sep 12, 2021 • edited Loading

Choose a reason for hiding this comment

dungdm93 Sep 13, 2021

Choose a reason for hiding this comment

hashhar commented May 30, 2021

dungdm93 commented Jun 2, 2021

buremba commented Aug 27, 2021 • edited Loading

hashhar left a comment

Choose a reason for hiding this comment

hashhar left a comment

Choose a reason for hiding this comment

esemeniuc commented Oct 13, 2021

hashhar left a comment

Choose a reason for hiding this comment

hashhar left a comment

Choose a reason for hiding this comment

hashhar commented Nov 25, 2021

dungdm93 commented Nov 25, 2021

hashhar commented Dec 2, 2021

dungdm93 commented Apr 2, 2021 •

edited

Loading

ebyhr Sep 3, 2021 •

edited

Loading

dungdm93 Sep 12, 2021 •

edited

Loading

buremba commented Aug 27, 2021 •

edited

Loading