-
-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add DbtTrinoHook #163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add DbtTrinoHook #163
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good, although I am running tests to confirm.
I do have one question: There is a plugin package for dbt: dbt-trino. If that plugin is required when using dbt normally it will likely also be required here.
I am not familiar with Trino but could you verify whether this package is required or that the hook works without it?
Ideally, we would have test coverage support to verify either way, but I will understand if we cannot reproduce a Trino environment in CI.
|
@tomasfarias This hook only builds the dbt profile dict (host/port/catalog/schema/etc.). It doesn’t talk to Trino or import the adapter. When dbt runs, it loads adapters via entry points; without dbt-trino installed, dbt will error (e.g. “Could not find adapter type ‘trino’”). I can add a small docs note, and (optionally) an extra in pyproject.toml so users can install it with: |
I don't think a docs note is necessary as we currently don't call out other specific adapters. Adding an extra dependency introduces additional build time, as we have more dependencies to resolve, and would require updating the lockfile. On the other hand, it can provide users a quicker way to install everything, and some runtime guarantees (although that only goes so far with Python). Anyways, all in all, I think this is fine, and we can release it as-is. We can always add dependencies later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI tests are passing.
|
@whitepowder I'll release this later today as airflow-dbt-python v3.1.0. Will default to your testing, as I have no resources at the moment to test Trino. Thanks for the work! |
Summary
Add a
DbtTrinoHooktoairflow_dbt_python/hooks/target.pyfor dbt-trino.airflow_conn_types=('trino',)conn.extradatabase→catalogfor dbt-trino; explicitcatalogoverrides the fallbacksession_properties,http_headers,role,client_tags,sourceExample Airflow connection (legacy extras-only)
{ "type": "trino", "method": "ldap", "user": "usert", "password": "pass", "host": "trino.example", "port": 443, "http_scheme": "https", "database": "example", "schema": "example", "verify": false }Tests
tests/hooks/test_trino.pyvalidates:database→catalogfallback and explicitcatalogoverrideNotes
get_dbt_details_from_connection.