-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adjust dbt profiles and schema macro to support prod/CI targets #44
Changes from 2 commits
1fd82dd
902f61d
62dc2b3
181e516
2f57842
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
dfsnow marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,22 @@ athena: | |
# "database" here corresponds to a Glue data catalog | ||
database: awsdatacatalog | ||
threads: 5 | ||
ci: | ||
type: athena | ||
s3_staging_dir: s3://ccao-dbt-athena-ci-us-east-1/results/ | ||
s3_data_dir: s3://ccao-dbt-athena-ci-us-east-1/data/ | ||
region_name: us-east-1 | ||
schema: dbt-test | ||
database: awsdatacatalog | ||
# Prefix all generated data by schema, so that we can delete it when the | ||
# PR is merged | ||
s3_data_naming: schema_table | ||
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am actually not totally sure if this works yet, since we're not yet using dbt to manage our CTAs, but the idea here is that eventually when dbt builds CTAs and stores them in S3 the |
||
threads: 5 | ||
prod: | ||
type: athena | ||
s3_staging_dir: s3://ccao-athena-results-us-east-1/ | ||
s3_data_dir: s3://ccao-athena-data-us-east-1/ | ||
region_name: us-east-1 | ||
schema: default | ||
database: awsdatacatalog | ||
threads: 5 |
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.
question (blocking): How does this work when the name of a branch includes an unexpected character (i.e. the
/
injeancochrane/28-data-catalog-add-production-profile-to-the-dbt-configuration
)?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.
Good question! I think we can replace special characters (and uppercase letters, which are not allowed in
dbt-athena
) using Jinja template filters. I'll take a stab at that now.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.
@dfsnow This spiraled a little bit, but I was able to make it work in 181e516! It required a new custom macro, but that led to me realize that we were starting to build a lot of logic into a collection of untested macros, so I took a minute to design a quick unit testing framework for our macros. Curious what you think!
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.
This is great! Nice work and definitely needed since this is getting more complex. I'm a dumb dumb and commented on a commit instead of the PR. Linking here: 181e516#r123228596