-
Notifications
You must be signed in to change notification settings - Fork 9
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
[Feature] Create option for customers to disable tag
and task_tag
source dependency models using variable configuration
#37
base: main
Are you sure you want to change the base?
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.
Looks and runs awesome, just a couple of suggestions.
- git: https://github.com/fivetran/dbt_asana_source.git | ||
revision: feature/tag-variable-configs | ||
warn-unpinned: false |
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.
obligatory reminder to switch
@@ -19,5 +19,7 @@ dbt deps | |||
dbt seed --target "$db" --full-refresh | |||
dbt run --target "$db" --full-refresh | |||
dbt test --target "$db" | |||
dbt run --vars '{asana__using_tags: false, asana__using_task_tags: false}' --target "$db" --full-refresh |
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.
dbt run --vars '{asana__using_tags: false, asana__using_task_tags: false}' --target "$db" --full-refresh | |
dbt run --vars '{asana__using_tags: false, asana__using_task_tags: false, asana_tag_identifier: missing, asana_task_tag_identifier: missing}' --target "$db" --full-refresh |
This way, the run will fail if the variables aren't working right
@@ -0,0 +1,45 @@ | |||
{{ config( |
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.
FYI this test is failing for me on tasks with multiple tags or projects because the string_agg
is aggregating them in different orders. So the rows are essentially the same, but the test thinks they're different because a, b, c
is not exactly c, b, a
.
I think this is fine for now/for this current ticket, but would you mind creating a FR ticket to apply predictable ordering to our string_agg
'd columns?
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.
PR Overview
This PR will address the following Issue/Feature: [#36]
This PR will result in the following new package version: v0.9.0
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
his release will introduce the following changes:
Breaking Changes
asana__using_tags
andasana__using_task_tags
to allow thetag
andtask_tag
source tables to be disabled. By default, these variables are set to True. (#37)int_asana__task_tags
andasana__tag
if either of the variables are set to False. This allows the downstream models to run even if the respective sourcetag
andtask_tag
tables don't exist. (#37)tags
andnumber_of_tags
inasana__task
if either of the variables are set to false.Under the Hood
asana__using_tags
andasana__using_task_tags
to thequickstart.yml
configuration to ensure when these source tables are not selected, these variables are set to false and the above changes are applied in Quickstart. (#37)asana__using_tags
andasana__using_task_tags
to our Buildkiterun_models.sh
script. (#37)integration_tests
to ensure no unexpected row changes occur in theasana__tag
andasana_task
models in development. (#37)Documentation
PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please share any and all of your validation steps:
New consistency tests passed for row comparisons
![Screenshot 2025-02-05 at 2 40 25 AM](https://private-user-images.githubusercontent.com/108772760/409949408-16b6f006-4dbd-4353-823f-f4a7ec23535f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2Mjk5MjMsIm5iZiI6MTczOTYyOTYyMywicGF0aCI6Ii8xMDg3NzI3NjAvNDA5OTQ5NDA4LTE2YjZmMDA2LTRkYmQtNDM1My04MjNmLWY0YTdlYzIzNTM1Zi5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjE1JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxNVQxNDI3MDNaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1lZTc1ODQzYWIxYjEyMWFjNjZlMjFjMjhhNjY0MmE0ZjFjNGQzNDg1MTZiN2I2NzQ5YTUwNmYwYjA4YTkxZDNhJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.VRAUfrviTxxUZjcWZ9ZAm1TGiF5zLUXcR33iv4lOfIA)
If you had to summarize this PR in an emoji, which would it be?
🪄