Refactor + resolve & persist docs errors#175
Conversation
….com/fivetran/dbt_jira into refactor/resolve-persist-docs-errors
fivetran-jamie
left a comment
fivetran-jamie
left a comment
There was a problem hiding this comment.
Looks great! Couple of in-line questions, plus this:
Should we remove the hard-coded story point logic from the timestamp_issue_field_history model and its intermediate as well? Things work fine with it still in there as it's dynamic, but maybe we want to totally clean it up
| cast(daily_issue_field_history.story_points as {{ dbt.type_numeric() }}) as story_points, | ||
| {% endif %} | ||
| {% if include_story_point_estimate %} | ||
| cast(daily_issue_field_history.story_point_estimate as {{ dbt.type_numeric() }}) as story_point_estimate, | ||
| {% endif %} |
There was a problem hiding this comment.
I see why it's ideal that these fields be numerics, but it makes me a little nervous to cast from string to numeric, at least without doing some cleaning first (that is, unless we can confirm that these values will always be formatted numerically)
Perhaps we'd wanna apply a macro to ensure only numeric characters are included before safely casting? We have a similar macro in Amazon Selling Partner.
| {% set issue_field_history_columns = var('issue_field_history_columns', []) | map('lower') | list %} | ||
| {% set include_story_points = 'story points' in issue_field_history_columns %} | ||
| {% set include_story_point_estimate = 'story point estimate' in issue_field_history_columns %} |
There was a problem hiding this comment.
Since this is repeated, I wonder if it's best to do this logic in the model itself and then pass include_story_points and include_story_point_estimate as macro parameters?
| select * | ||
| select * | ||
| from {{ ref('stg_jira__sprint_tmp') }} | ||
| where not coalesce(_fivetran_deleted, false) |
There was a problem hiding this comment.
could you move this to the end or the final CTE? Just in case anyone doesn't have _fivetran_deleted, the null version would get created in the fields CTE
2 participants
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
Submission Checklist
Changelog