airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

Published 20 hours ago • verified publisher icon airbytehq

🐛 Source Quickbooks: Replace Custom Components with Airbyte CDK

Open pabloescoder opened this issue 1 year ago • 3 comments

What

The manipulations that we were doing in the custom components.py file to create a CustomDateTimeCursor were unnecessary (they were done to access the nested date time field for the cursor back when we had no support for transformations natively in the cdk) However even after adding the transformation to bring the datetime value to the root level, we still kept the custom manipulations that caused this bug: https://github.com/airbytehq/airbyte/issues/44367

This PR scraps the custom logic and uses native Airbyte CDK features instead. The connector streams work now without any issues. Resolves https://github.com/airbytehq/airbyte/issues/44367

How

Use Airbyte CDK features to replace components.py.

Review guide

  1. manifest.yaml

User Impact

Fixes bug that prevented the streams from working & fetching data.

Can this PR be safely reverted and rolled back?

  • [X] YES 💚

pabloescoder avatar Aug 22 '24 19:08 pabloescoder

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2024 8:03am

vercel[bot] avatar Aug 22 '24 19:08 vercel[bot]

Regression tests run: https://github.com/airbytehq/airbyte/actions/runs/10515441620

natikgadzhi avatar Aug 22 '24 20:08 natikgadzhi

I've kicked off the regression tests.

The change in the manifest looks brutally large — I think that' because Builder reformatted it. We should be ok, but it's unreviewable.

Just noticed that :( If it helps - I made changes in the query request param for each stream (it used $parameters before) and added a record selector to every stream (each one has a different path based on the stream name), those were the major changes, apart from that did some minor changes to remove $parameters

pabloescoder avatar Aug 22 '24 21:08 pabloescoder

Kicked-off regression tests

natikgadzhi avatar Aug 27 '24 00:08 natikgadzhi

@pabloescoder can you please resolve the conflicts and merge master in? I'll kick the regression tests again.

natikgadzhi avatar Aug 28 '24 17:08 natikgadzhi

@natikgadzhi Fixed the merge conflicts & merged master, we can run regression tests now once all the pipelines complete running. Edit: CI has passed we can kick-off regression. Kicked off regression tests

pabloescoder avatar Aug 29 '24 18:08 pabloescoder

https://github.com/airbytehq/airbyte/issues/45911 Linking this issue too as it seems to be related.

hans-extenteam avatar Oct 01 '24 13:10 hans-extenteam