-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 TTL for BigQuery tables #2711
Merged
beckjake
merged 18 commits into
dbt-labs:dev/marian-anderson
from
kconvey:kconvey-model-ttl
Aug 19, 2020
Merged
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
81ab946
Add time_to_expiration
kconvey 6e06bd0
Add unit test
kconvey 099fea8
Add integration test
kconvey 108d843
Add newlines
kconvey 7aa8030
Fix test name
kconvey 25a869a
Fix unit test
kconvey 955f4ae
Add entry to CHANGELOG
kconvey a8d6691
Mock config better
kconvey eb35794
Fix table options string
kconvey 671a29f
plugins
kconvey 76aa8c7
Fix class name
kconvey 47ab741
Embed profile name
kconvey 3834805
Use injected sql from results
kconvey af118bc
Check stdout with --debug for actual ddl
kconvey 2562deb
Put --debug before run
kconvey b4a2ed6
Look for proper string
kconvey c28ffcd
merge with origin
kconvey 55813d9
hours_to_expiration
kconvey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1
test/integration/022_bigquery_test/adapter-specific-models/expiring_table.sql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
select 1 as id |
37 changes: 37 additions & 0 deletions
37
test/integration/022_bigquery_test/test_bigquery_adapter_specific.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
""""Test adapter specific config options.""" | ||
from test.integration.base import DBTIntegrationTest, use_profile | ||
import textwrap | ||
import yaml | ||
|
||
|
||
class TestBigqueryAdapterSpecific(DBTIntegrationTest): | ||
|
||
@property | ||
def schema(self): | ||
return "bigquery_test_022" | ||
|
||
@property | ||
def models(self): | ||
return "adapter-specific-models" | ||
|
||
@property | ||
def profile_config(self): | ||
return self.bigquery_profile() | ||
|
||
@property | ||
def project_config(self): | ||
return yaml.safe_load(textwrap.dedent('''\ | ||
config-version: 2 | ||
models: | ||
test: | ||
materialized: table | ||
expiring_table: | ||
time_to_expiration: 4 | ||
''')) | ||
|
||
@use_profile('bigquery') | ||
def test_bigquery_time_to_expiration(self): | ||
_, stdout = self.run_dbt_and_capture() | ||
self.assertIn( | ||
'expiration_timestamp: TIMESTAMP_ADD(CURRENT_TIMESTAMP(), INTERVAL ' | ||
'4 hour)', stdout) | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think the example I copied here was one that expected failure on the model, so the query would be dumped in
stdout
.I probably want to inspect
results
fromself.run_dbt()
, but could use a pointer to the compiled SQL within the results to do thisassertIn
(it's a little hard to decipher the schema sometimes). Let me know if that makes senseThere 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.
I think you want
results[index].node.injected_sql
. You can look for results by node name usingresults[index].node.name
.Also, don't feel at all obligated to do this, but because we use pytest for tests now you are free to use the (much more ergonomic, at least to me)
assert whatever in stdout
syntax.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.
@beckjake The error I got makes me think
injected_sql
isn't what I'm looking for in results.E AssertionError: 'expiration_timestamp: TIMESTAMP_ADD(CURRENT_TIMESTAMP(), INTERVAL 4 hour)' not found in 'select 1 as id'
This config adds the expiration_timestamp as part of the ddl, and if this was ddl, it should say something like
create or replace table as ...
. I can't remember if this is present in debug (which I believe just dumps the query), or where else the full ddl might be in the results. Any ideas?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.
Ah, now that I look at this more carefully, I think this will be in the output if you run with
--debug
, but not theinjected_sql
.injected_sql
contains the value that will end up as thesql
value in the materialization. But this change happens ultimately in thecreate_table_as
macro that's called from the materialization.It is, I suppose, always possible that we don't log all our queries on bigquery? That would be pretty bad behavior on our part.
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.
I ran this on a real project locally and it doesn't look like the ddl is anywhere in run_results.json, but it definitely is in the output with
--debug
.