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

fix: Use airbyte state message format + tracking. #18

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

JichaoS
Copy link
Contributor

@JichaoS JichaoS commented May 22, 2024

This change will fix incremental extraction for a few TAPs, like tap-amplitude and tap-s3.

  • Adds a new entry to state tracking json, 'airbyte_state'.
    • This new entry is populated on any invocation, and contains airbyte's way to track state as documented here: https://docs.airbyte.com/understanding-airbyte/database-data-catalog.
    • If this entry does not exist, we fallback to what was originally being done.
    • If this entry does exist, we use this data as the input state.json to airbyte's docker container.

This change will fix incremental extraction for a few TAPs, like tap-amplitude and tap-s3.

 - Adds a new entry to state tracking json, 'airbyte_state'.
   - This new entry is populated on any invocation, and contains airbyte's way to track state as documented here: https://docs.airbyte.com/understanding-airbyte/database-data-catalog.
   - If this entry does not exist, we fallback to what was originally being done.
   - If this entry does exist, we use this data as the input state.json to airbyte's docker container.
@JichaoS
Copy link
Contributor Author

JichaoS commented Jun 18, 2024

@z3z1ma let me know if you have any questions on this PR

@JichaoS
Copy link
Contributor Author

JichaoS commented Sep 5, 2024

@edgarrmondragon could you help getting the right pair of eyes on this?

@arianeschang
Copy link

arianeschang commented Sep 5, 2024

👋 I'm running into the same error that @JichaoS identified here with incremental runs of tap-googleads. Would love to see if I can be helpful in getting this fix merged! @z3z1ma would you be able to review this?

Copy link
Member

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if @SpaceCondor is affected by this or that this change at least doesn't break things for him

tap_airbyte/tap.py Outdated Show resolved Hide resolved
Co-authored-by: Edgar Ramírez Mondragón <[email protected]>
@edgarrmondragon
Copy link
Member

@JichaoS @arianeschang In the interesting of unblocking y'all I'm gonna merge this

@edgarrmondragon edgarrmondragon merged commit e573ee8 into MeltanoLabs:main Sep 6, 2024
3 checks passed
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