Skip to content
This repository was archived by the owner on Apr 8, 2024. It is now read-only.

feat: Do a single connection for whole model#557

Open
chamini2 wants to merge 1 commit intoact1from
act1-connection-test
Open

feat: Do a single connection for whole model#557
chamini2 wants to merge 1 commit intoact1from
act1-connection-test

Conversation

@chamini2
Copy link
Copy Markdown
Member

Tested it against jaffle_shop_with_fal. Please do more tests either way.

Copy link
Copy Markdown
Contributor

@isidentical isidentical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, fast PR @chamini2! Quick question, do you think it makes sense to add some except_handler stuff around the places where we actually write data since we now use a more unified connection?

Comment thread adapter/src/dbt/adapters/fal/adapter_support.py
Copy link
Copy Markdown
Contributor

@turbo1912 turbo1912 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chamini2 chamini2 force-pushed the act1-connection-test branch from 34043e8 to 532e94f Compare September 28, 2022 22:35
@chamini2
Copy link
Copy Markdown
Member Author

do you think it makes sense to add some except_handler stuff around the places where we actually write data since we now use a more unified connection?

Well, we don't really want to handle the exceptions, right? if it fails, it just fails and the model shows ERROR as status.

Or am I missing something?

@isidentical
Copy link
Copy Markdown
Contributor

Well, we don't really want to handle the exceptions, right? if it fails, it just fails and the model shows ERROR as status.

I meant let the DBT handle those exceptions, not us. E.g. calling db_adapter's exception_handler

@chamini2 chamini2 changed the title Do a single connection for whole model feat: Do a single connection for whole model Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants