Source Mailchimp - fix the way request params are built #20765

davydov-d · 2022-12-21T18:52:06Z

What

https://github.com/airbytehq/oncall/issues/1058

How

Do not use state as a source for request parameters. Use stream slices instead
Set test strictness level to high

davydov-d · 2022-12-21T18:52:40Z

/test connector=connectors/source-mailchimp

…te-issue-second-try

davydov-d · 2022-12-21T19:19:55Z

/test connector=connectors/source-mailchimp

davydov-d · 2022-12-21T19:27:10Z

/test connector=connectors/source-mailchimp

🕑 connectors/source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/3752232154
✅ connectors/source-mailchimp https://github.com/airbytehq/airbyte/actions/runs/3752232154
Python tests coverage:

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_mailchimp/__init__.py       2      0   100%
source_mailchimp/source.py        46      1    98%
source_mailchimp/streams.py      117      7    94%
--------------------------------------------------
TOTAL                            165      8    95%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-280, 288-301, 306-312, 319-330, 337-353
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1603    336    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [3] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
======================== 53 passed, 3 skipped in 51.12s ========================

artem1205 · 2023-01-09T19:48:00Z

/publish connector=connectors/source-mailchimp

🕑 Publishing the following connectors:
connectors/source-mailchimp
https://github.com/airbytehq/airbyte/actions/runs/3877203397

Connector	Did it publish?	Were definitions generated?
connectors/source-mailchimp	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

* 1058 source mailchimp - fix the way request params are built * auto-bump connector version Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

murphpdx · 2023-02-22T16:22:37Z

airbyte-integrations/connectors/source-mailchimp/source_mailchimp/streams.py

+            campaigns = Campaigns(authenticator=self.authenticator).read_records(sync_mode=SyncMode.full_refresh)
+        for campaign in campaigns:
+            slice_ = {"campaign_id": campaign["id"]}
+            cursor_value = stream_state.get(campaign["id"], {}).get(self.cursor_field)


It looks like cursor_field is set to timestamp. I know you have a sort_field of create_time but Mailchimp does not allow you to change the sorting. That means that you're going to lose a lot of records. The timestamp should stay the same the offset is what should be used to paginate. You will set the offset too offset = offset + pagesize; The since param should not change.
https://mailchimp.com/developer/marketing/docs/methods-parameters/#pagination
As you can see from the list-email-activity docs, there is no sort_field. I believe you're sort variable is just getting ignored.
https://mailchimp.com/developer/marketing/api/email-activity-reports/list-email-activity/

@murphpdx as for now there's just a single slice per campaign, this means we request all the data after the since date from the API and dont actually care if there is sorting or not. Can you confirm the bug still persists with you after upgrading to the latest version?

You're not paginating? It seems like you are here: https://github.com/airbytehq/airbyte/pull/20765/files#diff-56a4fde71ca00cdb2a92c3716de8b181115d431e07023b7881ee628aa382816cR46

We decided not to use airbyte last summer due in part to this bug. So I don't have the time to test it out for you.

Yes we are paginating and the since param remains the same throughout all the pages within one campaign. The thing is you are pointing to the stream_slices method which yields one slice per campaign and the pagination is made within one slice.

1058 source mailchimp - fix the way request params are built

5f493bd

octavia-squidington-iv added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/mailchimp labels Dec 21, 2022

davydov-d requested review from alafanechere, evantahler, lazebnyi and roman-yermilov-gl and removed request for evantahler and alafanechere December 21, 2022 18:53

Merge branch 'master' into ddavydov/#1058-source-mailchimp-stream-sta…

3894807

…te-issue-second-try

davydov-d requested a review from a team December 21, 2022 19:43

bazarnov approved these changes Dec 27, 2022

View reviewed changes

auto-bump connector version

e894f2c

octavia-squidington-iii temporarily deployed to more-secrets January 9, 2023 20:12 — with GitHub Actions Inactive

artem1205 merged commit 86157be into master Jan 9, 2023

artem1205 deleted the ddavydov/#1058-source-mailchimp-stream-state-issue-second-try branch January 9, 2023 21:30

octavia-squidington-iii mentioned this pull request Jan 12, 2023

Bump Airbyte version from 0.40.27 to 0.40.28 #21359

Merged

This was referenced Jan 16, 2023

Source Mailchimp: enable high test strictness level in SAT #19054

Closed

Source Mailchimp: enable high test strictness level in SAT #19167

Closed

davydov-d mentioned this pull request Feb 21, 2023

Source Mailchimp: email activity stream missing data #14673

Closed

murphpdx reviewed Feb 22, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source Mailchimp - fix the way request params are built #20765

Source Mailchimp - fix the way request params are built #20765

davydov-d commented Dec 21, 2022

davydov-d commented Dec 21, 2022

davydov-d commented Dec 21, 2022

davydov-d commented Dec 21, 2022 •

edited by github-actions bot

Loading

artem1205 commented Jan 9, 2023 •

edited by github-actions bot

Loading

murphpdx Feb 22, 2023 •

edited

Loading

davydov-d Feb 22, 2023 •

edited

Loading

murphpdx Feb 22, 2023 •

edited

Loading

murphpdx Feb 22, 2023

davydov-d Feb 22, 2023

Source Mailchimp - fix the way request params are built #20765

Source Mailchimp - fix the way request params are built #20765

Conversation

davydov-d commented Dec 21, 2022

What

How

davydov-d commented Dec 21, 2022

davydov-d commented Dec 21, 2022

davydov-d commented Dec 21, 2022 • edited by github-actions bot Loading

Build Passed

artem1205 commented Jan 9, 2023 • edited by github-actions bot Loading

murphpdx Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

davydov-d Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

murphpdx Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

murphpdx Feb 22, 2023

Choose a reason for hiding this comment

davydov-d Feb 22, 2023

Choose a reason for hiding this comment

davydov-d commented Dec 21, 2022 •

edited by github-actions bot

Loading

artem1205 commented Jan 9, 2023 •

edited by github-actions bot

Loading

murphpdx Feb 22, 2023 •

edited

Loading

davydov-d Feb 22, 2023 •

edited

Loading

murphpdx Feb 22, 2023 •

edited

Loading