-
Notifications
You must be signed in to change notification settings - Fork 6
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] Add tag
and task_tag
variable configs
#37
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 pretty good -- can you also adjust the source.yml to disable the tag
and task_tag
source tables based if their respective variables are set to False?
DECISIONLOG.md
Outdated
@@ -0,0 +1,13 @@ | |||
# Decision Log |
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.
we should remove the decision log now that we're just using the variables instead of does_table_exist()
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.
Removed.
Done! |
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.
@fivetran-jamie Ready for re-review!
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
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.
Approved with some minor comments.
CHANGELOG.md
Outdated
@@ -1,7 +1,16 @@ | |||
# dbt_asana_source version.version | |||
# dbt_asana_source v0.8.1 | |||
This release introduces the following updates. |
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 release introduces the following updates. | |
This release introduces the following updates: |
CHANGELOG.md
Outdated
|
||
## Documentation | ||
- Corrected references to connectors and connections in the README. ([#36](https://github.com/fivetran/dbt_asana_source/pull/36)) | ||
- Updated `src_asana.yml` with the above variable configurations to the source tables `tag` and `task_tag`. ([#37](https://github.com/fivetran/dbt_asana_source/pull/37)) |
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 would put this in Under the hood since it is functional.
PR Overview
This PR will address the following Issue/Feature: See issue #36 in
dbt_asana
This PR will result in the following new package version: v0.9.0
We are introducing variable configuration (entire models could be disabled if selected), so a breaking change seems like the best approach here.
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
Breaking Changes
asana__using_tags
andasana__using_task_tags
to disable thestg_asana__tag
andstg_asana__task_tag
models respectively. This allows the downstream models to run even if the respective sourcetag
andtask_tag
tables don't exist. For more information on how to configure refer to the README.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:
Tested the new variable configurations and confirmed the above models disabled when the variables were set to false.
If you had to summarize this PR in an emoji, which would it be?
🪄