-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Support DBT Manifests from Snowflake Stage #116
base: main
Are you sure you want to change the base?
Support DBT Manifests from Snowflake Stage #116
Conversation
Hey @kokorin 👋🏻 Quick heads up that this specific approach seems to not work on versions of dbt-core prior to 1.8.x. Would it be possible to see add some import logic to handle 1.6.0 and 1.7.0? |
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.
@kokorin This is a really great addition to the codebase! I think there's an incompatibility for dbt.mp_context
when working with older versions of dbt-core. Once that is sorted then this will be good to go.
Also, thank you for your patience on the review for this! I've been away for a bit to recharge, and it's nice to get back into the flow of things 😄
```yaml | ||
manifests: | ||
- name: project_name | ||
type: snowflake | ||
config: | ||
stage: stage_name # Stage name, can include Database/Schema | ||
stage_path: path/to/dbt/manifest.json # Path to manifest file in the stage | ||
``` |
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.
Praise: Thanks for adding an example for how to configure this manifest source.
|
||
from dbt.config.runtime import load_profile | ||
from dbt.flags import get_flags | ||
from dbt.mp_context import get_mp_context |
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.
Issue: I suspect that dbt-core API instability over time might require a different import for older versions of dbt-core. I'd recommend digging into dbt-core 1.6 and 1.7 to confirm.
# Import locally to not require dbt-snowflake to be installed | ||
from dbt.adapters.snowflake import SnowflakeAdapter |
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.
Thought: What happens here if adapters isn't installed? Should we throw an exception and a nice log line?
flags = get_flags() | ||
profile = load_profile( | ||
project_root=flags.PROJECT_DIR, | ||
cli_vars=flags.VARS, | ||
profile_name_override=flags.PROFILE, | ||
target_override=flags.TARGET, | ||
) | ||
adapter = SnowflakeAdapter(profile, get_mp_context()) |
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.
Praise: This is so dang clever. Fantastic work.
tmp_dir = tempfile.mkdtemp(prefix="dbt_loom_") | ||
# Snowflake needs '/' path separators | ||
tmp_dir_sf = tmp_dir.replace("\\", "/") | ||
|
||
with adapter.connection_named("dbt-loom"): | ||
get_query = f"get @{self.stage}/{self.stage_path} file://{tmp_dir_sf}/" | ||
response, table = adapter.connections.execute(get_query) | ||
if response.rows_affected == 0: | ||
raise Exception( | ||
f"Failed to get file {self.stage}/{self.stage_path}: {response}" | ||
) |
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.
Question: Am I correct in believing that this will download the file from the Snowflake stage into a temporary file?
Closes #109