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

🐛Source Pardot: Fix minor schema issues, improve data integrity by not using split-up interval, improve error retries #51040

Conversation

justbeez
Copy link
Contributor

What

This PR resolves several minor bugs related to the Pardot/Marketing Cloud Account Engagement connector:

  • Fix typing/deduping warnings by refining schemas
  • Remove split-up interval user input, which doesn't play nicely with pagination on very large Pardot accounts (leading to potential gaps in synced records)
  • Remove unneeded conditional parameters on custom_redirects
  • Adds a specific handler for the daily rate limits and adjust the default retry count/backoff strategy for other recoverable errors

How

Changes to all streams:

  1. Removed split-up interval option (which could lead to small data gaps on very high-volume accounts)

  2. Added a constant backoff when the daily API limit is hit

  3. Adjusted the default exponential backoff try count and factor to better handle transient errors seen in testing

Stream-specific updates:

  1. campaigns: added schema for missing fields to ensure type consistency across stream-level keys

  2. custom_redirects: fixed type of salesforceId (which is a string, unlike the Pardot native IDs); applied date-time type to createdAt; removed unneeded conditional query parameters

  3. email_clicks: switched created_at Airbyte type to timestamp_without_timezone (the v4 API doesn't return timezone)

  4. folders: applied date-time type to createdAt and updatedAt

  5. list_emails: applied date-time type to sentAt

  6. account: remove unneeded (but non-erroring) required field, createdAt

  7. custom_fields: Change fieldId to string type to prevent error; remove unneeded (but non-erroring) required field (createdAt)

  8. dynamic_contents: Make basedOnProspectApiFieldId string to avoid error

Review guide

Manifest and docs also updated accordingly.

User Impact

These are all non-breaking changes, and some fix errors that occur depending on the values of certain fields. Previously enabling these streams could result in a source error.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Jan 10, 2025

@justbeez is attempting to deploy a commit to the Airbyte Growth Team on Vercel.

A member of the Team first needs to authorize it.

@marcosmarxm
Copy link
Member

@DanyloGL and @ChristoGrab, could you please check the contribution made by Justin? I believe you are assisting him on Slack.

Copy link

vercel bot commented Jan 17, 2025

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 Jan 17, 2025 10:05pm

Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

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

I'm mostly happy! Let's see if basic CI runs well and merge.

@justbeez
Copy link
Contributor Author

@natikgadzhi Need anything from me to get this one landed?

@natikgadzhi
Copy link
Contributor

Just some verbal encouragement <3

@natikgadzhi
Copy link
Contributor

Hopefully kicked-off the CI, but will return to this later today and merge.

@ChristoGrab
Copy link
Contributor

ChristoGrab commented Jan 28, 2025

There was a mismatch between the manifest and base image versions that caused a build failure; just bumped the base image in metadata, will keep an eye on the results

@natikgadzhi
Copy link
Contributor

Alright, there is a build error because the manifest wants CDK version that is newer than what is in the base image.

We should update the base image to 6.22+ and that will work.

@natikgadzhi
Copy link
Contributor

Merging because the failure is complaining that we should have CATs

@natikgadzhi natikgadzhi merged commit c755fdd into airbytehq:master Jan 28, 2025
44 of 51 checks passed
btkcodedev pushed a commit that referenced this pull request Feb 3, 2025
…t using split-up interval, improve error retries (#51040)

Co-authored-by: Natik Gadzhi <natik@respawn.io>
Co-authored-by: ChristoGrab <christo.grab@gmail.com>
stephane-airbyte pushed a commit that referenced this pull request Feb 4, 2025
…t using split-up interval, improve error retries (#51040)

Co-authored-by: Natik Gadzhi <natik@respawn.io>
Co-authored-by: ChristoGrab <christo.grab@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/source/pardot
Projects
Development

Successfully merging this pull request may close these issues.

5 participants