-
Notifications
You must be signed in to change notification settings - Fork 29
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
Added new and open status duration metrics in business minutes #97
Added new and open status duration metrics in business minutes #97
Conversation
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.
Hi @Tim-Hoare thanks so much for taking the time to open this PR to incorporate the new and open duration business minutes!
After doing a cursory review of your PR I feel this looks good and is a natural extension of the existing duration business minutes. I will plan to do a more thorough review early next week and will plan to include these changes in our next release that will also include the changes from PR #94.
One request before jumping in further would be if you would be able to include the two new metrics you added as yml descriptions within the zendesk.yml file. This will ensure we properly include the field definitions for users to understand how to interpret these fields. Let me know if you have any other questions. Thanks again!
Additionally, if you wanted to give the branch with the PR #94 changes to confirm you see your expected output that would be a huge help! If you wanted, you can test that branch by using the following in your packages.yml packages:
- git: https://github.com/fivetran/dbt_zendesk.git
revision: bugfix/schedule-holiday-addition ## In place of your existing package version for Zendesk
warn-unpinned: false This branch essentially includes holidays within ticket schedules. If you do not use holidays then you should see no change. I am just looking to get confirmation on the expected behavior before rolling these changes out. Let me know if you are able to test. If not, no worries at all. Thanks 😄 |
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.
@Tim-Hoare thanks for working through the updates and contributing your changes to the package! I just made a few minor edits to ensure your branch will be able to be merged into our release branch.
We are planning to roll this out in the coming week. Thank you again for your contribution!
Are you a current Fivetran customer?
Yes: Tim Hoare, Data Scientist @ Codat
What change(s) does this PR introduce?
This adds a couple of new columns to the ticket metrics model, that are durations in business minutes that tickets are in the "new" and "open" status. These are counterparts to existing columns that are in calendar minutes
Did you update the CHANGELOG?
Does this PR introduce a breaking change?
I don't believe this is a breaking change as there are only new columns being added
Did you update the dbt_project.yml files with the version upgrade (please leverage standard semantic versioning)? (In both your main project and integration_tests)
Is this PR in response to a previously created Bug or Feature Request
How did you test the PR changes?
I made the changes locally and ran

dbt run --select zendesk
. All of these ran successfully and I can see that the new columns were added in the table in the database, with the desired outputSelect which warehouse(s) were used to test the PR
Provide an emoji that best describes your current mood
💃