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

Running a snapshot with missing required configurations results in uncaught Python exception #3381

Closed
1 of 5 tasks
jnatkins opened this issue May 21, 2021 · 2 comments · Fixed by #3385
Closed
1 of 5 tasks
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors! snapshots Issues related to dbt's snapshot functionality

Comments

@jnatkins
Copy link
Contributor

Describe the bug

In an attempt to educate myself on how snapshots work, I created a snapshot to test out, but neglected to realize that the configuration object was required. When I ran dbt snapshot, it failed with a fairly non-actionable error message:

2021-05-20T22:41:00.291112Z: uncaught python exception
'NoneType' object has no attribute 'lower' Code: -32000

Steps To Reproduce

  1. Create a snapshot without a config object, i.e.
{% snapshot snap_demo %}

select * from {{ ref('snapshot_demo_source') }}

{% endsnapshot %}
  1. Run dbt snapshot

Expected behavior

It would be helpful to get an error message that indicates what actually went wrong, and guides the user to provide required configurations

Screenshots and log output

If applicable, add screenshots or log output to help explain your problem.

System information

Which database are you using dbt with?

  • postgres
  • redshift
  • bigquery
  • snowflake
  • other (specify: ____________)

The output of dbt --version:

0.19.1

The operating system you're using:
MacOS

The output of python --version:

Additional context

Add any other context about the problem here.

@jnatkins jnatkins added bug Something isn't working triage labels May 21, 2021
@jtcohen6 jtcohen6 added snapshots Issues related to dbt's snapshot functionality and removed triage labels May 21, 2021
@jtcohen6
Copy link
Contributor

@jnatkins Such a good issue, and a great catch, thank you for opening!

I think there's a pretty straightforward resolution here, which is adding a check to the validate method of SnapshotConfig:

https:/fishtown-analytics/dbt/blob/e775f2b38e52e8b9f25f8ac98c942d22b2c8c55d/core/dbt/contracts/graph/model_config.py#L448-L485

This method already exists to ensure that snapshots with certain strategies (timestamp, check) also contain the required associated configurations. We should first just check to make sure the three required configurations are present. Something like:

        if not data.get('strategy') or not data.get('unique_key') or not data.get('target_schema'):
            raise ValidationError(
                "Snapshots must be configured with a 'strategy', 'unique_key', and 'target_schema'.")

So instead of 'NoneType' object has no attribute 'lower', you'd get:

Compilation Error in snapshot my_snap (snapshots/my_snap.sql)
  at path []: Snapshots must be configured with a 'strategy', 'unique_key', and 'target_schema'.

Whether those configs are set via in-file config() blocks, or in dbt_project.yml, doesn't much matter. They just need to be set one way or another.

I'm going to tag this as a good first issue. Is this a fix you (or anyone else) might be interested in contributing? :)

@jtcohen6 jtcohen6 added the good_first_issue Straightforward + self-contained changes, good for new contributors! label May 21, 2021
@jnatkins
Copy link
Contributor Author

@jtcohen6 I'd definitely be interested in contributing a fix here. I don't have a dev environment set up for dbt core yet, but I can probably scrape something together pretty quickly.

jnatkins added a commit to jnatkins/dbt that referenced this issue May 22, 2021
…caught Python exception (dbt-labs#3381)

* Running a snapshot with missing required configurations results in uncaught Python exception
jtcohen6 pushed a commit that referenced this issue May 24, 2021
…... (#3385)

* Running a snapshot with missing required configurations results in uncaught Python exception (#3381)

* Running a snapshot with missing required configurations results in uncaught Python exception

* Add fix details to CHANGELOG

* Update CHANGELOG.md

* Update invalid snapshot test with new/improved error message

* Improve changelog message and contributors addition
iknox-fa pushed a commit that referenced this issue Feb 8, 2022
…... (#3385)

* Running a snapshot with missing required configurations results in uncaught Python exception (#3381)

* Running a snapshot with missing required configurations results in uncaught Python exception

* Add fix details to CHANGELOG

* Update CHANGELOG.md

* Update invalid snapshot test with new/improved error message

* Improve changelog message and contributors addition

automatic commit by git-black, original commits:
  c0c487b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors! snapshots Issues related to dbt's snapshot functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants