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 Mailchimp: fix date-time format issue for start_date for email_activity stream #37572

Merged

Conversation

bazarnov
Copy link
Collaborator

What

Resolving: https://github.com/airbytehq/oncall/issues/4998

How

  • corrected the email_activity_stream. incremental_sync. datetime and email_activity_stream. incremental_sync.datetime_format format to handle the start_date value pattern correctly

User Impact

No impact, just fix.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Apr 25, 2024

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 Apr 25, 2024 0:37am

@bazarnov bazarnov self-assigned this Apr 25, 2024
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Apr 25, 2024
@bazarnov bazarnov marked this pull request as ready for review April 25, 2024 12:00
@octavia-squidington-iv octavia-squidington-iv requested review from a team April 25, 2024 12:01
@bazarnov bazarnov merged commit 6399307 into master Apr 25, 2024
35 checks passed
@bazarnov bazarnov deleted the baz/source/mailchimp/fix-start-date-format-in-manifest branch April 25, 2024 13:03
@NAjustin
Copy link
Contributor

Thanks for landing this fix; I was just about to submit it.

For what it's worth, I've also logged this as an issue with the Mailchimp API team, as this issue violates their documented ISO 8601 spec for that field. Specifically this was caused by not having a colon in the offset time (e.g. +0000 instead of +00:00), although Z/Zulu time notation works as well.

It also appears that this issue only happens when the values are encoded. Therefore:

#FAILS:
since=2024-01-01T00%3A00%3A00%2B0000

#WORKS (non-URL-encoded):
since=2024-01-01T00:00:00+0000

#WORKS (no timezone info):
since=2024-01-01T00%3A00%3A00

#WORKS (Z zulu time instead of +/- offset notation):
since=2024-01-01T00%3A00%3A00Z

#WORKS (zimezone offset with colon, e.g. +00:00):
since=2024-01-01T00%3A00%3A00%2B00%3A00

So the issue here is specifically with the timezone offset, and it not handling one of the 4 valid offset formats of ISO 8601 (Z, +HH:MM, +HHMM, +HH—each of which can also be negative with the exception of Zulu time). Airbyte uses the Pendulum library under the hood, and its ISO 8601, that library's ISO 8601 timestamp format doesn't use the colon.

I'm hoping Mailchimp will correct the issue so that it works with the encoded values AND without the colon, but for now I think this is an appropriate workaround to the issue. The did add an example that includes the colon to the docs, but hopefully they'll actually fix the issue for interoperability (and it's odd that they'll accept it when non-URL-encoded but not when properly encoded . . . definitely feels like a bug to me).

FVidalCarneiro pushed a commit to AgiData/airbyte that referenced this pull request May 2, 2024
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 connectors/source/mailchimp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants