dbt_github icon indicating copy to clipboard operation
dbt_github copied to clipboard

[Bug] int_github__pull_request_times.sql is dropping rows where no reviewers are explicitly requested

Open samkessaram opened this issue 1 year ago • 2 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the issue

The spine of the int_github__pull_request_times model is first_request_time, which is a CTE that inner joins pull requests to requested_reviewer_history. However, people can review pull requests without being explicitly requested, and then those pull requests can be merged. But because of the inner join, PRs without requested reviewers are dropped. This means that downstream, some merged PRs have a null merged_at timestamp.

Relevant error log or model output

No response

Expected behavior

Merged PRs should have a non-null merged_at timestamp.

Possible solution

Change the inner join to be a left join.

dbt Project configurations

n/a

Package versions

  - package: fivetran/github
    version: 0.7.0

What database are you using dbt with?

snowflake

How are you running this dbt package?

dbt Core™

dbt Version

1.8.0

Additional Context

No response

Are you willing to open a PR to help address this issue?

  • [X] Yes.
  • [ ] Yes, but I will need assistance.
  • [ ] No.

samkessaram avatar Sep 10 '24 20:09 samkessaram

Hi @samkessaram ! Thanks for taking a look into this, your approach appears to make sense to implement in a future sprint for our team.

Give us some time to validate that this approach won't have any unintended downstream impacts, and we will be in touch shortly.

fivetran-avinash avatar Sep 11 '24 17:09 fivetran-avinash

@samkessaram I just wanted to provide an update that we will plan to further review this and consider integrating into the upcoming GitHub release in the last sprint of October. Apologies for the delay, but you can expect us share more updates then and likely will plan to include in an October release.

fivetran-joemarkiewicz avatar Sep 26 '24 19:09 fivetran-joemarkiewicz

Hi @samkessaram! Thanks for your patience. I'm taking a look at your PR now and all appears to look good. I plan to merge this into a larger release branch where we can apply some of our standard documentation updates and validations, at which point we plan to merge it into a wider release. Look out for it in the coming weeks!

fivetran-avinash avatar Oct 18 '24 16:10 fivetran-avinash

Hi @samkessaram , thanks for your patience! We have gone ahead and released the latest version with your proposed fixes on the dbt hub.

Let us know if there are any questions or thoughts you have! We will go ahead and close this issue out.

fivetran-avinash avatar Oct 23 '24 22:10 fivetran-avinash

Sweet, thanks all!

samkessaram avatar Oct 24 '24 13:10 samkessaram