trinodb / trino-python-client

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

Pass additional_headers to cursor.execute #265

Closed benrifkind closed 1 year ago

benrifkind commented 1 year ago

Description

This adds the ability to specify additional_http_headers in Cursor.execute.

Non-technical explanation

Adds the ability to specify additional HTTP headers when calling Cursor.execute. Since this is already available in trino.client.TrinoQuery.execute it is just a matter of exposing it as an argument in Cursor.execute.

This also unblocks me for #264 without making a change that it appears was already decided against. In short, this enables me to add tags on a per query basis even though Trino does not technically support it. See https://github.com/trinodb/trino-python-client/pull/154/files/9dc1e697f2141c1fc89083654b599015679bc456#r795281894

Release notes

(x) 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. ( ) Release notes are required, with the following suggested text:

* Expose additional_headers in Cursor.execute. #264 
cla-bot[bot] commented 1 year ago

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

benrifkind commented 1 year ago

For sure. The actual issue I am trying to solve is #264. I want to be able to specify tags on a per query basis. Another way I could do this is just create a new connection for each query but that seems like overkill to me.

Edit: Sorry I read your response more carefully. It looks like you are indeed suggesting that I create a connection per query. I'll just do that. Thanks for the response.

hashhar commented 1 year ago

I'll create a new issue to see if there are things other than client_tags which a user might want to set on per-query basis. If we can come up with a good rationale for that then we can think about how to make this change in a way that's not limited to client_tags and not as open-ended as additional_headers.

Yes - for your use-case I'd suggest using new Connection/Cursor per query should be fine.

Thanks for linking the use-case you had in mind.