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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

partition fix #47

Merged
merged 14 commits into from
Oct 5, 2021
Merged

partition fix #47

merged 14 commits into from
Oct 5, 2021

Conversation

fivetran-jamie
Copy link
Contributor

Are you a current Fivetran customer?

work at fivetran

What change(s) does this PR introduce?

  • add variable to limit field history to the last X years
  • add variable to extend field history Y months
  • cap ticket field history at its closure date (prior to adding the extension)
  • a lil README formatting
  • adding instructions regarding the extension + limit variables to the README

Does this PR introduce a breaking change?

  • Yes (please provide breaking change details below.)
  • No (please provide explanation how the change is non breaking below.)

yes in that now ticket field histories are capped at the close date, so like running a full refresh with this code will remove some rows that existed before (of post-closure tickets). no in that that may not be considered "breaking" since the tickets are closed and also in that the ticket_field_history_extension_months variable can be used to persist this behavior

thoughts? @fivetran-joemarkiewicz @fivetran-reneeli

Is this PR in response to a previously created Issue

How did you test the PR changes?

  • CircleCi
  • Other (please provide additional testing details below)

Select which warehouse(s) were used to test the PR

  • BigQuery
  • Redshift
  • Snowflake
  • Postgres
  • Databricks
  • Other (provide details below)

Provide an emoji that best describes your current mood

馃悎

Feedback

We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.

Copy link
Contributor

@fivetran-reneeli fivetran-reneeli left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

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

@fivetran-jamie this PR looks great. I have one small request for rebasing the branch and we will include this in a breaking change for v0.7.0.

However, I set the review for requesting changes, but in actuality I would really just like for you to walk @fivetran-reneeli and I through the process of how this will help cleanup the zendesk__ticket_field_history model (ie. stop producing records after so many days of being closed). We can do this during standup tomorrow.

We can pick a ticket or two tomorrow and see with these changes how if the ticket is closed for X days then it will stop creating records, and also how it will impact records that are still open. With all the changes that will be applied to this package I want our team to get a better understanding of how all these moving pieces will work together!

Thanks!

dbt_project.yml Outdated Show resolved Hide resolved
@fivetran-jamie fivetran-jamie changed the base branch from release/v0.6.latest to release/v0.7.latest October 5, 2021 20:47
@fivetran-jamie
Copy link
Contributor Author

pre-change:
image
last day is today

post-change:
image
last day is 2020-02-027!

Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

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

Looks great!! Thanks so much for working on this @fivetran-jamie! 馃

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.

None yet

3 participants