-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 Mailchimp: Add expected records #32226
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
…ehq/airbyte into christo/mailchimp-sandbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Minor comments left
airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py
Outdated
Show resolved
Hide resolved
…imp/streams.py Co-authored-by: Serhii Lazebnyi <53845333+lazebnyi@users.noreply.github.com>
Co-authored-by: Serhii Lazebnyi <53845333+lazebnyi@users.noreply.github.com>
…ehq/airbyte into christo/mailchimp-sandbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Nice! I confirm that the expected records don't have PII (we made some fake test accounts)
What
Currently, we are reading from our internal Mailchimp account for CAT tests, so expected_records can't be populated in order to avoid sharing internal data. We do have an old sandbox account that has now been reactivated and can be safely used for testing purposes.
How
type
property in Campaigns streamclicks.last_click
property. This causes validation errors due to the declareddate-time
format. Added logic to exclude the property if it is an empty string.