trino
trino copied to clipboard
Trino CLI: support --extra-header parameter
Description
Adds an --extra-header flag to the Trino CLI to allow for passing arbitrary HTTP headers to Trino requests.
Additional context and related issues
This can be useful for all sort of header-y things, including passing the X-Trino-Routing-Group
header for the presto-gateway, or adding specific values needed by other fanciful architectures. :)
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:
# Section
* Add a `--extra-header` argument to the trino-cli to support sending arbitrary HTTP headers to Trino
cc @electrum @hashhar , this is a more general rework of #15737, let me know your thoughts! 🙏
Is there an equivalent of this in the JDBC driver?
That's a good question - from a look at the docs, it appears not... At least not in Trino. Presto's JDBC client does support a customHeaders
parameter since August 2021 (https://github.com/prestodb/presto/commit/a6f1d0fb9c4c90af8ed5ed3ad971bacba343a62c), but Trino doesn't.
This was discussed in https://github.com/trinodb/trino/issues/3179 but not implemented at the time, as user impersonation was implemented with explicit headers and covered the use-case of the original reporter.
We're not using JDBC here so we only need this feature for the Trino CLI, but having support for extra headers in the code makes implementing this for JDBC easier.
I would be surprised if this get approved. Trino generally does not allow flags to arbitrarily change core infrastructure like this. When we add features we like to start from the core problem and come up with a specific solution for that problem.
@dain our usecase is we use presto-gateway to load-balance and front multiple trino clusters. we divide them into groups and would like to be able to use the trino-cli to issue queries to a specific group of clusters.
I understand presto-gateway isn't core trino, but it is a fairly used system and more importantly this design pattern of having a gateway/load-balancer to allow for blue/green or rolling deployments of trino without causing downtime is necessary for us (and many others I'd imagine).
We'd like all our tooling that issues requests to trino (including the trino-cli) to use this gateway and to be able to select a specific group of clusters.
how would you suggest this be approached? should we modify presto-gateway to use a specific existing HTTP API client http header ? the only i see that is somewhat a candidate is X-Trino-Client-Tags
and even that.. feels like overloading it for a different purpose
Especially as Trino's own website lists the presto-gateway
in its list of resources, so it would make sense (imho!) to be able to support it from the official CLI 🙏
Is this PR dead or will it make it into the code base?
Is this PR dead or will it make it into the code base?
Currently that depends on @Pluies to update and resolve the conflicts. Subsequently it can be reviewed again.
@mosabua thanks for the ping, this was flying under my radar - updating now!
PR updated and ready for re-review 👍
@mosabua , there's an example in docs/src/main/sphinx/client/cli.md
, do you have another thing in mind for documentation?
@electrum @dain @nineinchnick - can we decide on a direction for this. Does it maybe fit in given that we are working towards trino-gateway ?
@msf could you suggest a name for a new header to be used for routing?
@msf could you suggest a name for a new header to be used for routing?
I would suggest X-Trino-Routing-Group
, like mentioned above, which is already used by: https://github.com/trinodb/trino-gateway and other open source projects: https://github.com/search?q=X-Trino-Routing-Group&type=code
We're not using JDBC here so we only need this feature for the Trino CLI, but having support for extra headers in the code makes implementing this for JDBC easier.
If CLI supports X-Trino-Routing-Group for gateway project, I think it would be great for trino client REST API (with protocol headers) & JDBC also be updated and support same header for same purpose.
FYI, I just tested the approach of using X-Trino-Client-Tags
with trino-gateway which works brilliantly so either way works :)
Gateway provides routing-rules engine option for those circumstance where it is hard to add X-Trino-Routing-Group
request header (JDBC, Trino CLI etc.)
-
By default, Trino Gateway reads the
X-Trino-Routing-Grouprequest header to route requests. If this header is not specified, requests are sent to default routing group (adhoc).
- https://github.com/trinodb/trino-gateway/blob/main/docs/routing-rules.md
I opened a similar change in https://github.com/trinodb/trino-go-client/pull/91 and was pointed to this PR.
Depending on the environment and other systems at play, custom headers are a go-to way for passing information between systems. I have use cases for passing an Authorization
header with each request, as well as other random headers such as transaction ID and trace ID. I would love to see this happen as it opens many doors for developers with little downside.
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua
:wave: @pluies @electrum - this PR has become inactive. We hope you are still interested in working on it. Please let us know, and we can try to get reviewers to help with that.
We're working on closing out old and inactive PRs, so if you're too busy or this has too many merge conflicts to be worth picking back up, we'll be making another pass to close it out in a few weeks.
@bitsondatadev @colebow @mosabua Any chance the team at Trino can get some eyes on this?
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua
I requested a review/decision on next steps from @electrum
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time.