-
Notifications
You must be signed in to change notification settings - Fork 15
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
Bug/string agg limit #133
Bug/string agg limit #133
Changes from 8 commits
f4dc4db
86dc929
196dbb6
88400a9
cf7d12a
c9fe823
655c552
6ec7632
5097e52
1f12065
a3c5468
e654e99
d26b848
27c9c11
a49fa75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,7 +66,7 @@ Include the following jira package version in your `packages.yml` file: | |
```yaml | ||
packages: | ||
- package: fivetran/jira | ||
version: [">=0.18.0", "<0.19.0"] | ||
version: [">=0.19.0", "<0.20.0"] | ||
|
||
``` | ||
### Step 3: Define database and schema variables | ||
|
@@ -82,11 +82,11 @@ vars: | |
Your Jira connector may not sync every table that this package expects. If you do not have the `SPRINT`, `COMPONENT`, or `VERSION` tables synced, add the respective variables to your root `dbt_project.yml` file. Additionally, if you want to remove comment aggregations from your `jira__issue_enhanced` model, add the `jira_include_comments` variable to your root `dbt_project.yml`: | ||
```yml | ||
vars: | ||
jira_using_sprints: false # Disable if you do not have the sprint table or do not want sprint-related metrics reported | ||
jira_using_components: false # Disable if you do not have the component table or do not want component-related metrics reported | ||
jira_using_versions: false # Disable if you do not have the versions table or do not want versions-related metrics reported | ||
jira_using_priorities: false # disable if you are not using priorities in Jira | ||
jira_include_comments: false # This package aggregates issue comments so that you have a single view of all your comments in the jira__issue_enhanced table. This can cause limit errors if you have a large dataset. Disable to remove this functionality. | ||
jira_using_sprints: false # Enabled by default. Disable if you do not have the sprint table or do not want sprint-related metrics reported. | ||
jira_using_components: false # Enabled by default. Disable if you do not have the component table or do not want component-related metrics reported. | ||
jira_using_versions: false # Enabled by default. Disable if you do not have the versions table or do not want versions-related metrics reported. | ||
jira_using_priorities: false # Enabled by default. Disable if you are not using priorities in Jira. | ||
jira_include_comments: false # Disabled by default for Redshift, enabled by default for other supported warehouses. This package aggregates issue comments so that you have a single view of all your comments in the jira__issue_enhanced table. This can cause limit errors if you have a large dataset. Disable to remove this functionality. | ||
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. Can we also document the new variable for conversations 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. Ahh yes updated! |
||
``` | ||
### (Optional) Step 5: Additional configurations | ||
|
||
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
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. Small note, but I noticed the end model results in no conversation or count_comments because of this join. It seems that since we don't have any Would you be able to update the seed data to have at least one match so we can verify results in the future. 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. 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. 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. Very interesting 🤔 thanks for showing the results! It must have been a mismatch on my end with using old seed data in place of your updates. This is all good to me! |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
|
||
{{ config( | ||
tags="fivetran_validations", | ||
enabled=var('fivetran_validation_tests_enabled', false) | ||
) }} | ||
|
||
{% set exclude_columns = ['avg_age_currently_open_seconds', 'avg_age_currently_open_assigned_seconds', 'median_age_currently_open_seconds', 'median_age_currently_open_assigned_seconds', 'epics', 'components'] %} | ||
fivetran-jamie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
with prod as ( | ||
select {{ dbt_utils.star(from=ref('jira__project_enhanced'), except=exclude_columns) }} | ||
from {{ target.schema }}_jira_prod.jira__project_enhanced | ||
), | ||
|
||
dev as ( | ||
select {{ dbt_utils.star(from=ref('jira__project_enhanced'), except=exclude_columns) }} | ||
from {{ target.schema }}_jira_dev.jira__project_enhanced | ||
), | ||
|
||
prod_not_in_dev as ( | ||
-- rows from prod not found in dev | ||
select * from prod | ||
except distinct | ||
select * from dev | ||
), | ||
|
||
dev_not_in_prod as ( | ||
-- rows from dev not found in prod | ||
select * from dev | ||
except distinct | ||
select * from prod | ||
), | ||
|
||
final as ( | ||
select | ||
*, | ||
'from prod' as source | ||
from prod_not_in_dev | ||
|
||
union all -- union since we only care if rows are produced | ||
|
||
select | ||
*, | ||
'from dev' as source | ||
from dev_not_in_prod | ||
) | ||
|
||
select * | ||
from final |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
|
||
{{ config( | ||
tags="fivetran_validations", | ||
enabled=var('fivetran_validation_tests_enabled', false) | ||
) }} | ||
|
||
{% set exclude_columns = ['avg_age_currently_open_seconds', 'median_age_currently_open_seconds', 'projects'] %} | ||
fivetran-jamie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
with prod as ( | ||
select {{ dbt_utils.star(from=ref('jira__user_enhanced'), except=exclude_columns) }} | ||
from {{ target.schema }}_jira_prod.jira__user_enhanced | ||
), | ||
|
||
dev as ( | ||
select {{ dbt_utils.star(from=ref('jira__user_enhanced'), except=exclude_columns) }} | ||
from {{ target.schema }}_jira_dev.jira__user_enhanced | ||
), | ||
|
||
prod_not_in_dev as ( | ||
-- rows from prod not found in dev | ||
select * from prod | ||
except distinct | ||
select * from dev | ||
), | ||
|
||
dev_not_in_prod as ( | ||
-- rows from dev not found in prod | ||
select * from dev | ||
except distinct | ||
select * from prod | ||
), | ||
|
||
final as ( | ||
select | ||
*, | ||
'from prod' as source | ||
from prod_not_in_dev | ||
|
||
union all -- union since we only care if rows are produced | ||
|
||
select | ||
*, | ||
'from dev' as source | ||
from dev_not_in_prod | ||
) | ||
|
||
select * | ||
from final |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
{{ config(enabled=var('jira_include_comments', True)) }} | ||
{{ config(enabled=var('jira_include_comments', False if target.type == 'redshift' else True)) }} | ||
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. Taking a closer look at this now, since you were able to identify that the string_agg was likely the only issue I think we may be going overboard with turning off the entirety of this model by default for Redshift users. Especially when there is still some valuable information here (with the Additionally, if we disable these models by default then we don't end up using the Can we instead adjust this model in particular to not disable it entirely, but just provide the conditional on the string_agg to include or not include based on the 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. We would then need to make slight adjustments in the 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. @fivetran-joemarkiewicz Thanks for the explanation! I updated accordingly, and it's ready for re-review! |
||
|
||
with comment as ( | ||
|
||
|
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.
Just to clarify that it's renamed
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.
Updated!