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

[Feature] Create option for customers to disable tag and task_tag source dependency models using variable configuration #37

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fivetran-avinash
Copy link
Contributor

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

  • Introduces variables asana__using_tags and asana__using_task_tags to allow the tag and task_tag source tables to be disabled. By default, these variables are set to True. (#37)
  • This will disable the tables int_asana__task_tags and asana__tag if either of the variables are set to False. This allows the downstream models to run even if the respective source tag and task_tag tables don't exist. (#37)
  • This will exclude the fields tags and number_of_tags in asana__task if either of the variables are set to false.
  • For more information on how to configure these variables, refer to the README. (#37)

Under the Hood

  • Added asana__using_tags and asana__using_task_tags to the quickstart.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)
  • Added False configurations for asana__using_tags and asana__using_task_tags to our Buildkite run_models.sh script. (#37)
  • Added consistency tests within integration_tests to ensure no unexpected row changes occur in the asana__tag and asana_task models in development. (#37)

Documentation

  • Added Quickstart model counts to README. (#35)
  • Corrected references to connectors and connections in the README. (#35)

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

  • dbt run –full-refresh && dbt test
  • [NA] dbt run (if incremental models are present) && dbt test

Before marking this PR as "ready for review" the following have been applied:

  • The appropriate issue has been linked, tagged, and properly assigned
  • All necessary documentation and version upgrades have been applied
  • docs were regenerated (unless this PR does not include any code or yml updates)
  • BuildKite integration tests are passing
  • [x\ Detailed validation steps have been provided below

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

If you had to summarize this PR in an emoji, which would it be?

🪄

Copy link
Collaborator

@fivetran-jamie fivetran-jamie left a 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.

Comment on lines +4 to +6
- git: https://github.com/fivetran/dbt_asana_source.git
revision: feature/tag-variable-configs
warn-unpinned: false
Copy link
Collaborator

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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(
Copy link
Collaborator

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

2 participants