bug/too-many-partitions #131
Conversation
There was a problem hiding this comment.
@fivetran-catfritz great work on these updates. I checked the run in a few different scenarios as well as with the validation tests on both full refresh and incremental versions and this is looking great!
I do have a few documentation change requests before this will be ready for approval. Let me know once those are applied and the docs regenerated and this should be ready for approval and release review.
models/intermediate/field_history/int_jira__issue_calendar_spine.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@fivetran-joemarkiewicz Thank you for the review! I made the updates and also regenerated docs. I will do the final incremental run on Wednesday for final approval.
| @@ -15,7 +15,7 @@ | |||
|
|
|||
| {% if is_incremental() %} | |||
| -- set max date_day with lookback as a variable for multiple uses | |||
| {% set max_date_day = jira.jira_lookback(from_date='max(date_day)', datepart='day', interval=var('lookback_window', 3)) %} | |||
| {% set max_date_week = jira.jira_lookback(from_date='max(date_week)', datepart='week', interval=var('lookback_window', 1)) %} | |||
There was a problem hiding this comment.
Do we need to update the lookback window README section (and thus the CHANGELOG) that the default is now 1 week rather than 3 days? Or is it just for specific cases?
There was a problem hiding this comment.
Very good catch--definitely yes. I have updated!
There was a problem hiding this comment.
@fivetran-catfritz Great job! A few comments before release.
Co-authored-by: Avinash Kunnath <[email protected]>
There was a problem hiding this comment.
Thank you @fivetran-avinash! I have updated from your comments, so it's ready for review.
| @@ -15,7 +15,7 @@ | |||
|
|
|||
| {% if is_incremental() %} | |||
| -- set max date_day with lookback as a variable for multiple uses | |||
| {% set max_date_day = jira.jira_lookback(from_date='max(date_day)', datepart='day', interval=var('lookback_window', 3)) %} | |||
| {% set max_date_week = jira.jira_lookback(from_date='max(date_week)', datepart='week', interval=var('lookback_window', 1)) %} | |||
There was a problem hiding this comment.
Very good catch--definitely yes. I have updated!
There was a problem hiding this comment.
@fivetran-catfritz Nice! Whenever Buildkite starts passing, lgtm!
The only additional suggestion I have is to add a "Documentation Update" section to link to the new README updates so it's easy for users to see the new Lookback Window section.
PR Overview
This PR will address the following Issue/Feature:
This PR will result in the following new package version:
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
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:
Full Refresh
Incremental Run
If you had to summarize this PR in an emoji, which would it be?
💃