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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Union source data from multiple QuickBooks instances #26

Merged

Conversation

ligfx
Copy link
Contributor

@ligfx ligfx commented Apr 6, 2022

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?

  • Yes

Does this PR introduce a breaking change?

  • No
    Backwards compatible

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)

  • Yes

Is this PR in response to a previously created Bug or Feature Request

How did you test the PR changes?

  • Local

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

  • Snowflake

Provide an emoji that best describes your current mood
🐻

@ligfx
Copy link
Contributor Author

ligfx commented Apr 6, 2022

Based off of fivetran/dbt_xero_source#11

@fivetran-joemarkiewicz
Copy link
Contributor

fivetran-joemarkiewicz commented Apr 8, 2022

@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
Copy link
Contributor Author

ligfx commented Apr 12, 2022

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 ligfx force-pushed the union_source_data branch from f948435 to cb7d72f Compare April 19, 2022 19:45
@ligfx ligfx force-pushed the union_source_data branch from cb7d72f to a146e10 Compare April 19, 2022 19:46
@ligfx
Copy link
Contributor Author

ligfx commented Apr 19, 2022

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

@fivetran-joemarkiewicz fivetran-joemarkiewicz changed the base branch from main to feature/quickbooks-updates-project January 19, 2023 18:49
@fivetran-joemarkiewicz fivetran-joemarkiewicz changed the base branch from feature/quickbooks-updates-project to feature/customer-union January 19, 2023 22:55
@fivetran-joemarkiewicz
Copy link
Contributor

@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
Copy link
Contributor Author

ligfx commented Jan 20, 2023 via email

@fivetran-joemarkiewicz fivetran-joemarkiewicz changed the base branch from feature/customer-union to customer/union-merge January 20, 2023 15:01
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.

Thanks so much for these contributions @ligfx! I unfortunately was unable to make changes directly to your branch due to permission issues. However, I will fold these into a customer branch on the main repo and will fold them into our next release from there.

Thank you again so much for helping to contribute to our QuickBooks package. This union feature will be one many will be able to take full advantage of!

@fivetran-joemarkiewicz fivetran-joemarkiewicz merged commit 527da6e into fivetran:customer/union-merge Jan 20, 2023
@fivetran-avinash
Copy link
Contributor

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!

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.

3 participants