Skip to content
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

fix: External table models with missing fields now produce an error #47

Merged
merged 8 commits into from Mar 11, 2022

Conversation

ericf-firebolt
Copy link
Contributor

@ericf-firebolt ericf-firebolt commented Mar 1, 2022

Description

Previously, external table models with missing or misspelled regex or data_type fields caused indecipherable errors. Those errors are now clear. In addition, drop/create on external tables only occurs if variable is set: dbt run-operation stage_external_sources --vars "ext_full_refresh: true". Otherwise, table is skipped.

Fixes

dbt seeds doesn't truncate tables before running

Checklist

  • I have run this code in development and it appears to resolve the stated issue.
  • This PR includes tests, or tests are not required/relevant for this PR.
  • I have updated CHANGELOG.md and added information about my change.
  • If this PR requires a new PyPI release I have bumped the version number.
  • I have pulled/merged from the main branch if there are merge conflicts.
  • I have verified that this PR contains only code changes relevant to this PR.

@ericf-firebolt ericf-firebolt self-assigned this Mar 1, 2022
@ericf-firebolt ericf-firebolt changed the title Fix: External table models with missing fields now produce an error fix: External table models with missing fields now produce an error Mar 1, 2022
@octavianzarzu30
Copy link
Contributor

hey @ericf-firebolt

  1. when running
    % dbt run-operation stage_external_sources
    I get:
10:25:29  Running with dbt=1.0.1
10:25:29  Unable to do partial parsing because config vars, config profile, or config target have changed
10:25:30  12:25:30 + 1 of 1 START external source s3.raw_customers
10:25:34  12:25:34 + 1 of 1 (1) DROP TABLE IF EXISTS raw_customers
10:25:34  12:25:34 + 1 of 1 (1) OK
10:25:34  12:25:34 + 1 of 1 (2) --    CREATE EXTERNAL TABLE raw_customers ("id" in...  
10:25:35  12:25:35 + 1 of 1 (2) OK

Shouldn't it skip in this case?

  1. Using false for the flag, I get a runtime error:
    % dbt run-operation stage_external_sources --vars "ext_full_refresh: false"
10:25:45  Running with dbt=1.0.1
10:25:45  Unable to do partial parsing because config vars, config profile, or config target have changed
10:25:46  12:25:46 + 1 of 1 START external source s3.raw_customers
10:25:50  12:25:50 + 1 of 1 (1) --    CREATE EXTERNAL TABLE raw_customers ("id" in...  
10:25:50  Encountered an error while running operation: Runtime Error
  Runtime Error
    Error executing query:
    View/Table 'raw_customers' already exists.
  1. Using true for the flag returns correctly (table is first dropped):
    % dbt run-operation stage_external_sources --vars "ext_full_refresh: false"
10:29:33  Running with dbt=1.0.1
10:29:33  Unable to do partial parsing because config vars, config profile, or config target have changed
10:29:34  12:29:34 + 1 of 1 START external source s3.raw_customers
10:29:38  12:29:38 + 1 of 1 (1) DROP TABLE IF EXISTS raw_customers
10:29:38  12:29:38 + 1 of 1 (1) OK
10:29:38  12:29:38 + 1 of 1 (2) --    CREATE EXTERNAL TABLE raw_customers ("id" in...  
10:29:39  12:29:39 + 1 of 1 (2) OK
  1. I'm not quite sure why the Unable to do partial parsing because config vars, config profile, or config target have changed warning keeps popping up

@ericf-firebolt
Copy link
Contributor Author

ericf-firebolt commented Mar 2, 2022

@octavianzarzu30, I'm unable to recreate your issues. Are you sure you're running the most current version of the branch?

As to the first question though, where you run dbt run-operation stage_external_sources, did the table exist before you did that? Even though it defaults to false, if the table doesn't exist it will create it.

That warning message comes from dbt and is output when the --vars value changes from run to run—at least that's the behavior for me. That's the case even when ext_refresh_full is left off after having been set to false before.

@octavianzarzu30
Copy link
Contributor

Yep - pip3 install git+https://github.com/firebolt-db/dbt-firebolt.git@external-table-edits
and ran - the table existed before. Let's check tomorrow. If it's not reproducible on your end, then I'm good to go. @todd-firebolt can you give it a try as well?

Copy link
Collaborator

@stepansergeevitch stepansergeevitch left a comment

Choose a reason for hiding this comment

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

lgtm

dbt/adapters/firebolt/impl.py Outdated Show resolved Hide resolved
+ "')"
)
# Todo: See if name is a required field.
if 'name' in partition and partition['name'] is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same nit here. Validate first and then do what is needed

Copy link
Collaborator

@stepansergeevitch stepansergeevitch left a comment

Choose a reason for hiding this comment

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

I get an internal error

(.venv) ~/code/jaffle_shop_firebolt [main|…1]$ dbt run-operation stage_external_sources
12:11:04  Running with dbt=1.0.3
12:11:04  1 of 1 START external source s3.raw_customers
12:11:09  1 of 1 (1) DROP TABLE IF EXISTS raw_customers
12:13:20  Encountered an error while running operation: Runtime Error
  Runtime Error
    Error executing query:
    Query failed - Internal execution error

@sonarcloud
Copy link

sonarcloud bot commented Mar 11, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@ericf-firebolt ericf-firebolt merged commit 96b7eda into main Mar 11, 2022
@ericf-firebolt ericf-firebolt deleted the external-table-edits branch March 11, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants