-
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] Incorrect Historical Value when Two Updates on the Same Day #90
Comments
hey there, really appreciate your thoroughness! int_jira__daily_field_history, which the pivot model references, should be looking at only the last Sprint value for an issue for each day i actually think the issue is that you have two different field ids for Sprint ( |
Our environment was customized over the years so I wouldn't be surprised if they changed a setting that resulted in two fields being used behind the scenes. With that said we only have one Sprint field I can see in the UI and its showing both of the toggled sprints. Thanks for pointing me to that earlier table. From a consistency perspective perhaps 'int_jira__daily_field_history' should be using the field name instead of the ID for the partition? Alternatively the pivot logic would need to be changed but I'd think these go hand in hand. row_number() over ( |
i think for most most folks, partitioning by the for your case however, i'd recommend overriding the |
Thanks Jamie! Your guidance on how the package was intended to work makes that easier. I'll make one last appeal to a broader fix but likely proceed to making a fix on our end. I understand this situation might not occur regularly but it could happen to others. The risk is without rigorous testing it would go unnoticed and result in incorrect reporting. If it is discovered later than people will waste cycles trying to fix it. As a data practitioner that subscribes to 'No Broken Windows' - this bothers me. There are multiple ways to solve the problem and I think this would ultimately be a fix that is relatively easy to implement now as we know about it while preventing future headache for others. In any case- thank you for the support. |
Yeah that's very fair, as there have honestly been quite a few discussions around Next sprint (in a week), the team and I take some time to think this through. can't promise anything (except that we'll seriously explore it 🤠 ), but I want to find a solution that can address your (and probably others') issue without adding too much complexity to the package. |
Hey @fivetran-jamie - Just curious where the team landed on this. Appreciate all of the support again. We'll likely be trying out of a few of the prebuilt transformation packages soon. |
hey hey -- so we will indeed be adding the option to use this will probably be achieved through a Thanks again for pushing on this @jon-goldberg 🙌 In my investigations this past sprint, I realized that the package definitely assumes field names to be unique, which is recommended by Jira but clearly not always the case. I think this will be helpful to lots of folks! Timeline-wise, I'll be working on this next sprint (starts tomorrow) |
Wooo-hoo! Awesome. Thanks Jamie for the support 💪. |
hey hey @jon-goldberg -- i've got a working branch if you have time to test it out! to do so, you'd replace the jira part of your packages:
- git: https://github.com/fivetran/dbt_jira.git
revision: feature/switch-field-grain and then in your vars:
jira_field_grain: 'field_name' # default value is still field_id FYI i am still recreating your use case (ie duplicate fields) in our own Jira sandbox environment, so would really appreciate you testing this out if you've got bandwidth! |
@fivetran-jamie Apologies for the delay - we were pulled into some project related work. Philip and I just tested - this appears to have addressed the problems we faced before but it did cause a failing test for some of our duplicate fields. I am guessing this is to be expected and we should remove the test. I spot checked a field of the problem issue ids and the final table matches what I see in Jira. unique_int_jira__combine_field_histories_combined_history_id Thank you so much for the help! |
no problem! oh huh that's interesting... we do use the currently, |
@jon-goldberg could you try this out and see if everything still ties out (and if the test failure is removed)? # packages.yml
packages:
- git: https://github.com/fivetran/dbt_jira.git
revision: releases/v0.13.latest updated the branch FYI |
heads up we're planning to release these updates tomorrow if you @jon-goldberg (or anyone creeping on this thread 👁️ 👁️) have a moment today or tomorrow to retest 🌟 |
Hi All, the updates @fivetran-jamie highlighted above are now live in the latest version of the package ( As such, I will close out this issue. Please feel free to reopen if the issue persists on your end. Thanks again! |
Thanks Jamie and Joe! Philip didn't have time to update our codebase to use the new packages so I could test but he is planning to do so next week. Appreciate the support with this. |
Is there an existing issue for this?
Describe the issue
int_jira__pivot_daily_field_history has an issue where there are two transactions on the same day one of which is null. The scenario is as follows.
case when field_value is null then 'is_null' else field_value end as field_value,
I imagine this affects all other historical fields where there are two transactions on the same day. The code should be changed to select the last transaction on a day and use those values over others. Its not as simple as ignoring the nulls as those could be the last transaction in certain cases. This is the offending code block.
Relevant error log or model output
No response
Expected behavior
dbt Project configurations
N/A
Package versions
N/A
What database are you using dbt with?
postgres
dbt Version
N/A
Additional Context
Are you willing to open a PR to help address this issue?
The text was updated successfully, but these errors were encountered: