elementary-data
[ELE-489] Support dbt-trino connector
Is your feature request related to a problem? Please describe.
At $WORK, I heavily leverage on Trino to use multiple data sources within my DBT projects (using the dbt-trino connector).
It's also quite useful for storing DBT's test results in a different database than the analytical one (by using the --store-failures flag combined with the +schema and +database config).
Describe the solution you'd like
Elementary is IMHO a better solution than the raw --store-failures flag and it could be great to be able to make it work with the dbt-trino connector.
Describe alternatives you've considered
For now, none :) I'm still using the --store-failures flag but without the proper visualization, I can't make it very useful for the rest of the teams.
Additional context Nope.
Would you be willing to contribute this feature?
Totally! I started to dig a little bit (I seached postgres__ to see what kind of macro needs to be implemented) but I need guidance on how to start and how to test.
Hi @radium226! We would be happy to support your effort in making Elementary compatible with Trino. To be honest I'm not familiar with it so I don't know how hard it would be.
Generally speaking, we implemented every platform-specific functionality using the adapter.dispatch functionality, as dbt recommends. You can see an example in this macro.
However, where there was a dbt_utils macro that we could use, we did. I do see utils in the dbt-trino adapter, so it looks promising in that sense.
Anyway, You can see here a workaround we did for a missing util in Spark.
I think you should approach it gradually - Step 1 - Add support for uploading dbt artifacts and run results (in the dbt package). Step 2 - Add support in the CLI for Slack alerts and UI generation. Step 3 - Add support for data anomaly detection test (the most complex and platform-specific part of the code right now).
You can see here a guide for testing: https://docs.elementary-data.com/general/contributions#contributing-to-the-dbt-package
Hello @radium226 Happy to hear you benefit from Trino! (dbt-trino PM here).
This is the first time I see a request for Elementary integration, I will take a closer look.
Thanks for reaching out. Piotr
Hello @leniartek and @Maayan-s,
In the coming weeks, I will be dedicated to implementing this integration, as we need Elementary working with Trino.
@leniartek, I would be very grateful if you have already identified any requirements related to this integration and could share them with me, so I can have a better idea of what I need to focus on.
In any case, I am available if you require help with anything.
Hi @ndrluis !
That is great to hear. FYI that in the upcoming Elementary release we are going to add Athena support thanks to the contribution of @artem-garmash with the help of @nicor88. I know that Athena v3 is based on Trino so I'm wondering if there will be common code to both.
Also a couple of notes:
- Please see our contribution guide here
- Please see especially the note about integration tests in the dbt package - in particular these should help you to verify various Elementary features work as expected.
- I'll be more than happy to jump on a call to help you get started / assist in any other way.
Cheers, Itamar
I'm unsure on the status of other peoples work, but I've been wanting to use Elementary with Trino as well so I threw up the two required pull requests:
- https://github.com/elementary-data/dbt-data-reliability/pull/652
- https://github.com/elementary-data/elementary/pull/1378
Hopefully they won't need many changes and we get them merged soon 👀
@Tomme this is exciting, Elementary supporting trino would be a huge win. What needs to happen to get this into the mainline?
@haritamar, @Maayan-s: maybe do you have insight about how to move on with the 2 PRs of @Tomme? I think we're all waiting for it :)
Amazing, thanks a lot!
Le mar. 13 févr. 2024, 06:55, Itamar Hartstein @.***> a écrit :
Hi @Tomme https://github.com/Tomme @radium226 https://github.com/radium226 , I will review this week and update here!
— Reply to this email directly, view it on GitHub https://github.com/elementary-data/elementary/issues/739#issuecomment-1940471219, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQFYUGOYSHICOUXX2AAOU3YTL55DAVCNFSM6AAAAAAVRDZQUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBQGQ3TCMRRHE . You are receiving this because you were mentioned.Message ID: @.***>
Hi @Firstero @radium226 , Apologies for the delay, more urgent things came up, but hoping to get it done this week.
I hope I'm not too pushy... But do you have any news @haritamar? I can't wait to try it, that's why I'm asking :)
Hi @Tomme @radium226 @mtk12 ! Apologies for the super long delay here, thank you for your patience.
I've actually reviewed both PRs, and also tested locally and in our CI. Created a PR on top of the dbt package PR that includes CI tests and a small fix.
Overall it looks really great, left a question here and a comment here that is actually on our end to fix as it's a fix in FE code.
I believe we'll be able to merge it in the next few days. and then have it in the next official version.
Cheers, Itamar
This is one of the best news of the week so far :) Thanks a lot!
Le mar. 9 avr. 2024, 21:40, Itamar Hartstein @.***> a écrit :
Hi @Tomme https://github.com/Tomme @radium226 https://github.com/radium226 @mtk12 https://github.com/mtk12 ! Apologies for the super long delay here, thank you for your patience.
I've actually reviewed both PRs, and also tested locally and in our CI. Created a PR https://github.com/elementary-data/dbt-data-reliability/pull/687 on top of the dbt package PR that includes CI tests and a small fix.
Overall it looks really great, left a question here https://github.com/elementary-data/dbt-data-reliability/pull/652/files#r1558189324 and a comment here https://github.com/elementary-data/elementary/pull/1378/files#r1558190813 that is actually on our end to fix as it's a fix in FE code.
I believe we'll be able to merge it in the next few days. and then have it in the next official version.
Cheers, Itamar
— Reply to this email directly, view it on GitHub https://github.com/elementary-data/elementary/issues/739#issuecomment-2045931435, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQFYUBCKEWVEEDICO2T32LY4Q72PAVCNFSM6AAAAAAVRDZQUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBVHEZTCNBTGU . You are receiving this because you were mentioned.Message ID: @.***>
@haritamar Thank you for taking a look Itamar and getting the PR merged 🥳
If you have any questions please feel free to send me a message!
@haritamar this is huge for me personally, thank you so much for getting this into the codebase
Hi @Tomme , thanks a lot for the contribution!
Yup just got it merged :) I made some small changes:
- The main one is adding Trino tests to our dbt package CI.
- Made the temp tables name have a maximum of 128 chars rather than 255, since it seemed to be an issue with the Hive connector.
- Reduced the minimum version to 1.5.0 to make it a bit less strict - lmk if you think that's an issue, it seemed to work for me.
I'm still curious about this (e.g. making should_commit always false for Trino) - though all tests pass and it doesn't affect other adapters so didn't feel like a blocker to me in any case.
I'll update when this gets into an official version!
Hello everyone,
I have opened a PR to fix a problem with Trino and Elementary. I just copied the Athena behavior.
You can check it out here: https://github.com/elementary-data/dbt-data-reliability/pull/735