-
Notifications
You must be signed in to change notification settings - Fork 16
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
Release/snowplow-web/0.5.0 #53
Conversation
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.
LGTM assuming your Redshift tests went ok and the temp commit is removed 👍
2c2f39a
to
fadda79
Compare
For clarity I am holding off merging this as although all tests pass on Redshift, dbt is throwing warnings for the optional page view context models such as: [WARNING]: Test 'test.snowplow_web.not_null_snowplow_web_pv_iab_page_view_id.858bd03daa' (models/page_views/scratch/page_views_scratch.yml) depends on a node named 'snowplow_web_pv_iab' which is disabled This is because the model These page view context models are only to be run on Redshift and Postgres and as such I have set the tests to only run for these databases: dbt-snowplow-web/models/page_views/scratch/page_views_scratch.yml Lines 261 to 264 in fadda79
I have tried disabling the test if the model is also disabled like beneath but unfortunately the vars value doesnt seem to be being read in from the - unique:
enabled: "{{ var('snowplow__enable_iab') and target.type in ['redshift', 'postgres'] | as_bool() }}" I have reached out to dbt for help |
I think the issue I outlined above has been raised already: Added my example in the hope of speeding up the fix. I think I have a workaround - will add now. |
All tests for Redshift have passed locally. @paulboocock - will fix up latest commit and merge PR once you have cast your eye over the fix. |
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.
Thats a fun little bug. The fix looks good to me though.
5c32d56
to
4305dfe
Compare
Running through Redshift tests. Will confirm results