Replace Dropwizard with Airlift

oneonestar commented 2 months ago

Description

Resolve #41

Commits cannot be built and tested individually. This is caused by the mass dependency conflict between Dropwizard and Airlift. It is impossible to replace Dropwizard part by part and keep each commit buildable. Also, it will be very difficult to review if all changes are squashed into one commit.

TODO in this PR

[x] Fix docker build & health-check
[x] Find out if there are better ways to handle the configuration (yaml vs .properties)
[x] HTTPS support

Follow-up later in other PRs

Merge requestRouter, applicationConnectors and adminConnectors ports (#382)
Update docs (#382)
Refactor and make the use of dependency injection consistent

How to run in Intellij

VM options: -Xmx1g -Djdk.attach.allowAttachSelf=true Main class: io.trino.gateway.ha.HaGatewayLauncher Argument: docs/quickstart-config.yaml Working directory: $ProjectFileDir$

Release notes

(x) Release notes are required, with the following suggested text:

* Replace Dropwizard with Airlift
* (TODO) Document the incompatible changes

Chaho12 commented 3 weeks ago

I assume this is ready for PR right?

oneonestar commented 3 weeks ago

I assume this is ready for PR right?

Yes. the all-or-nothing changes are done in this PR. Reviews are welcome. I'm still working on merging different ports into one. The changes are big and they can be tested individually. I'd like to create another PR for that. Docs will come afterward.

oneonestar commented 2 weeks ago

I've address the UI path issue in #382 with the following changes:

Path change for WebUI

The root path for Trino Gateway WebUI has been changed from / to /tgw. By default, access to / will be redirected to /tgw. This behavior can be changed by adding / to extraWhitelistPaths, which will route / to Trino.

Format of `extraWhitelistPaths`

The path setting in extraWhitelistPaths is now a regex matches the full URI in request. The old version will forward requests with a URI prefix that matches any path in extraWhitelistPaths. The new version takes regexes from extraWhitelistPaths and forwards only those requests with a URI that exactly matches any of the regexes. This changes is required because in the old version, / will match and forward every request.

Be sure to use single-quoted strings so that escaping is not required. The following configurations are equal.

Old config:

extraWhitelistPaths:
    - "/ui"
    - "/v1/custom"

New config:

extraWhitelistPaths:
    - '/ui.*'
    - '/v1/custom.*'

rdsarvar commented 2 weeks ago

I've address the UI path issue in #382 with the following changes:

Path change for WebUI

The root path for Trino Gateway WebUI has been changed from / to /tgw. By default, access to / will be redirected to /tgw. This behavior can be changed by adding / to extraWhitelistPaths, which will route / to Trino.

Format of extraWhitelistPaths

The path setting in extraWhitelistPaths is now a regex matches the full URI in request. The old version will forward requests with a URI prefix that matches any path in extraWhitelistPaths. The new version takes regexes from extraWhitelistPaths and forwards only those requests with a URI that exactly matches any of the regexes. This changes is required because in the old version, / will match and forward every request.

Be sure to use single-quoted strings so that escaping is not required. The following configurations are equal.

Old config:
extraWhitelistPaths:
    - "/ui"
    - "/v1/custom"
New config:
extraWhitelistPaths:
    - '/ui.*'
    - '/v1/custom.*'

is the naming of tgw up for discussion? personally i prefer the explicit noun based naming /trino-gateway/vX where you'd abstract away the clustering from the user with <?more than 1 gateway?>.trinodb.<org domain>.com/trino-gateway/

is the issue with it the potential lengths of the URL? i see added readability as tgw isn't a well known abbreviation

mosabua commented 2 weeks ago

We will need to ensure we add a breaking release notes entry with upgrade info (or link to docs with the info would be even better).

Also I think the longer path would actually be nice ..

oneonestar commented 2 weeks ago

I'm fine with /trino-gateway. I am not aware of any practical limitations for having a longer URL path. We can finalize the decision in today's meeting.

wendigo commented 2 weeks ago

Let's land this and improve in follow-ups.

wendigo commented 2 weeks ago

@mosabua thoughts?

trinodb / trino-gateway