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

Normalization integration test #873

Merged
merged 9 commits into from
Nov 10, 2020
Merged

Conversation

ChristopheDuong
Copy link
Contributor

@ChristopheDuong ChristopheDuong commented Nov 10, 2020

What

Enable normalization in destination integration tests

Here i remove the _normalized schema suffix but add it to the table name suffix instead until #845 defines proper _raw naming for tables produced before normalization.
There is also a method in TestDestination.assertEquivalentMessages that needs to be better implemented since I didn't know how to properly compare expected messages and actual ones...

(Messages from normalized tables contains a few extra metadata columns that fails the tests with assertSameMessages)

{HKD: 13,2, date: 2020-11-01}
vs
{emitted_at: 2020-11-01, HKD: 13,2, date: 2020-11-01, normalized_at: 2020-11-01, _exchange_hashid: 2141sfas21aaz2}

Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

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

Nice! Let's merge it. What's left We need to do the same for BQ and snowflake integration tests, right?

f"cast({MACRO_START} json_extract_scalar('{json_col}', {current}) {MACRO_END} as {MACRO_START} dbt_utils.type_int()"
+ f" {MACRO_END}) as {MACRO_START} adapter.quote_as_configured('{name}', 'identifier') {MACRO_END}"
return "cast({} as {}) as {}".format(
jinja_call(f"json_extract_scalar('{json_col}', {current})"),
Copy link
Contributor

Choose a reason for hiding this comment

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

nice. so we didn't end up having to do anything integration specific in the dbt stuff. we were able to use the jinja macros!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I had to write ugly DBT SQL (with macro everywhere) so it's the most generic possible and we handle destination specific in the DBT macros

@@ -1,7 +1,33 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

How did it get in initially? Shouldn't have broke the build?

Copy link
Contributor

Choose a reason for hiding this comment

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

@cgardens cgardens merged commit e4168fa into master Nov 10, 2020
@swyxio swyxio deleted the chris/normalization-integration-test branch October 12, 2022 18:26
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.

3 participants