Complete transition to Trino #64
Conversation
| I used the code to execute some queries on our Trino351 cluster. Works. |
hashhar left a comment
hashhar left a comment There was a problem hiding this comment.
Thanks for picking this up. ❤️
There are some non-rename changes in the same commit. I'd like if you could extract them into a separate commit at the end.
Also, some changes requested.
| def test_transaction_multiple(presto_connection_with_transaction): | ||
| with presto_connection_with_transaction as connection: | ||
| def test_transaction_multiple(trino_connection_with_transaction): | ||
| with trino_connection_with_transaction as connection: |
There was a problem hiding this comment.
Split this change into two commits. One with just the rename and the next one with the refactoring.
Thanks for catching this.
There was a problem hiding this comment.
Nevermind about this, I misread the diff.
trino/constants.py Outdated
| | ||
| DEFAULT_PORT = 8080 | ||
| DEFAULT_SOURCE = "presto-python-client" | ||
| DEFAULT_SOURCE = "Trino-python-client" |
There was a problem hiding this comment.
fixed. sharp eyes! :)
thanks
including: identifier, HTTP-header, imports, tests, documentation, etc. Co-authored-by: Ashhar Hasan <hashhar_dev@outlook.com>
hashhar left a comment
hashhar left a comment There was a problem hiding this comment.
Looks good to me.
Would love a second pair of eyes - specifically the HTTP header changes.
| Merged, thanks! |
| Is there a way to configure the header values? It seems that the sever I'm using is returning headers with the presto value, such as |
| @callado4 I would recommend upgrading your cluster. Trino Python client doesn't provide the way to configure those values. |
| @callado4 While you upgrade the cluster you can use |
5 participants
replaced almost every occurrence of "presto" with "trino".
README.md migrated, too -- except the Presto book link
changed default version of Trino for integration tests to 351