search

fivetran / dbt_quickbooks_source

Data models for QuickBooks built using dbt.
https://fivetran.github.io/dbt_quickbooks_source/
Apache License 2.0
5 stars 18 forks source link

Union source data from multiple QuickBooks instances #26

Closed ligfx closed 1 year ago

ligfx commented 2 years ago

Are you a current Fivetran customer? Michael Maltese, Solution Engineer, Sigma Computing

What change(s) does this PR introduce? Add ability to union from multiple data sources

Did you update the CHANGELOG?

Does this PR introduce a breaking change?

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?

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

Provide an emoji that best describes your current mood :bear:

ligfx commented 2 years ago

Based off of https://github.com/fivetran/dbt_xero_source/pull/11

fivetran-joemarkiewicz commented 2 years ago

@ligfx this is an amazing feature addition to the package! I also really appreciate how you took the time in scaffolding the changes from our Xero integration here as well. 💯

At first glance these changes look great. I will take time over the next few weeks doing a deeper review of this PR. Additionally, we will want to update the dbt_quickbooks package to account for the union feature as well. Since there are quite a few joins that take place in that package I imagine the work there will be quite extensive and may take more time.

My team can plan for the work in the modeling package over the month or so. Otherwise, if you would like to attempt a PR on the modeling package I would be happy to assist in any way I can 😄. Otherwise, I will continue to share updates on the progress of this PR and the future PR here.

Again I want to thank you so much for putting the time into opening this PR. It is because of contributors like yourself that these packages continue to be improved upon! I look forward for when we get to integrate this into our package! 🏅

ligfx commented 2 years ago

Awesome, thanks @fivetran-joemarkiewicz . I've started taking a peek at the dbt_quickbooks package too, though as you mentioned that's much more extensive.

This also rebases reasonably cleanly on top of the casting PR (just some whitespace errors and the changelog). I can update this PR once that one gets merged in.

ligfx commented 2 years ago

Rebased on top of the latest main now that the casting fixes have landed.

fivetran-joemarkiewicz commented 1 year ago

@ligfx I wanted to chime back in here as we are looking to incorporate this union feature into the next release of the QuickBooks package!

We have integrated a few pieces already within our working branch, but want to include a number of your contributions. In order to do this I will need to edit your branch to address a number of merge conflicts. Is this branch by chance being used in production at all? If not, would you mind if I made a few edits to allow for this to be successfully merged into our updates branch and accept your contributions?

ligfx commented 1 year ago

Hi Joe,

Glad to hear it! Please, go ahead and make any modifications necessary.

On Thu, Jan 19, 2023 at 6:05 PM Joe Markiewicz @.***> wrote:

@ligfx https://github.com/ligfx I wanted to chime back in here as we are looking to incorporate this union feature into the next release of the QuickBooks package!

We have integrated a few pieces already within our working branch, but want to include a number of your contributions. In order to do this I will need to edit your branch to address a number of merge conflicts. Is this branch by chance being used in production at all? If not, would you mind if I made a few edits to allow for this to be successfully merged into our updates branch and accept your contributions?

— Reply to this email directly, view it on GitHub https://github.com/fivetran/dbt_quickbooks_source/pull/26#issuecomment-1397723701, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAERBLN6A3Y47HIXFSBZ4CLWTHCBXANCNFSM5SULZSGQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Michael Maltese (209) 404-5755 linkedin.com/in/mmaltese New York, NY

fivetran-avinash commented 1 year ago

Thanks again for all your help @ligfx!

We have applied your updates to our dbt_quickbooks_source package in our latest release. Definitely take a look at the new package and see if you're able to union your source data now! Thanks for all your contributions!