Mailchimp upsert contacts/update tags in batch #124

VSydor · 2023-04-19T15:55:39Z

No description provided.

src/main/java/com/impactupgrade/nucleus/client/MailchimpClient.java

brmeyer · 2023-04-28T01:31:06Z

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

@@ -32,6 +47,10 @@ public class MailchimpEmailService extends AbstractEmailService {

  private static final Logger log = LogManager.getLogger(MailchimpEmailService.class);

+  private static final Integer BATCH_REQUEST_OPERATIONS_SIZE = 50;


Remind me on how we landed on 50 as the size? MC API limit?

No - just decided to go with 50. Can be any reasonable value.

Any chance you've seen any defined limits in the doc? If not, 50 is still a gigantic improvement, but curious how high we can crank this up...

According to example described here:
https://mailchimp.com/developer/marketing/guides/run-async-requests-batch-endpoint/

It is possible to have 1000 records in single batch (!)

Minor: I would still use a smaller amount as a batch size so that we don't wait too long till contacts are inserted/updated before processing the tags

Using 500 as a batch size (replicated from GetMembers method)

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

brmeyer · 2023-04-28T01:42:38Z

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

@@ -222,6 +383,63 @@ protected Map<String, Object> getCustomFields(String listId, CrmContact crmConta
    return customFieldMap;
  }

+  protected Map<String, Map<String, Object>> getCustomFields(String listId, List<CrmContact> crmContacts, MailchimpClient mailchimpClient,
+      EnvironmentConfig.EmailPlatform mailchimpConfig) throws Exception {


Was there something that made this additional method necessary, instead of calling the original getCustomFields from a loop?

The idea is to create custom fields for all contacts at once (instead of for each contact separately).
But please let me know if I should switch to original method.

Right, I just mean most of this code (and the one in the other comment I made) duplicates the original, right? Helper methods might DRY it up?

Yes, absolutely - the general idea was to adopt existing methods to batch-like processing (ex, get all tags in 1 shot)
and then remove original ones (after/during code-review phase).
Please let me know which original ones you want to keep (getCustomFields?) and I will clean it all up.

Switched back to original method (wrapped in for-each)

brmeyer · 2023-04-28T01:43:18Z

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

+                                   List<CrmContact> crmContacts,
+                                   Map<String, List<String>> activeTags,
+                                   Map<String, List<String>> inactiveTags,
+                                   MailchimpClient mailchimpClient) {


Ditto -- was this additional method needed, as opposed to using the original updateTags called from a loop?

The idea is to call update-in-batch for all contacts at once (instead of many update-tags calls)

brmeyer · 2023-04-28T01:44:12Z

Functionally, looks great! Just some design pattern questions. I'm going to give this a test run on Save the Storks to see how it handles a sync of 55k+...

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

src/main/java/com/impactupgrade/nucleus/client/MailchimpClient.java

brmeyer · 2023-05-05T02:29:05Z

@VSydor Hey! This is working well so far -- giving it a shot with our own HubSpot->Mailchimp sync. One thing I noticed that isn't necessarily hurting anything, but would be good to fix:

[WARN ] 2023-05-04 22:24:22.316 [Thread-13] MailchimpEmailService - Failed to deserialize batch operations! Cannot deserialize value of type `java.lang.String` from Object value (token `JsonToken.START_OBJECT`)
 at [Source: (String)"{"status_code":200,"response":{"list_id":"9c168e9869","_links":[{"targetSchema":"https://us1.api.mailchimp.com/schema/3.0/Definitions/Lists/Members/Response.json","method":"GET","rel":"self","href":"https://us1.api.mailchimp.com/3.0/lists/9c168e9869/members/1ddf93db4d70f42a3a78cf49e3f26fd2"},{"schema":"https://us1.api.mailchimp.com/schema/3.0/Paths/Lists/Members/Collection.json","targetSchema":"https://us1.api.mailchimp.com/schema/3.0/Definitions/Lists/Members/CollectionResponse.json","method":""[truncated 3064 chars]; line: 1, column: 2578] (through reference chain: com.impactupgrade.nucleus.client.MailchimpClient$BatchOperation["response"]->com.impactupgrade.nucleus.client.MailchimpClient$BatchOperationResponse["merge_fields"]->java.util.LinkedHashMap["ADDRESS"])

brmeyer · 2023-05-05T02:29:36Z

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

+        Integer newAttemptCount = attemptCount + 1;
+        batchOperations = getBatchOperations(mailchimpClient, mailchimpConfig, batchStatusId, newAttemptCount);
+      } else {
+        log.info("Batch '{}' finished! (finished/total) {}/{}", batchStatus.finished_operations, batchStatus.total_operations);


@VSydor This line looks to be missing an argument -- the last {} is showing up in the logs

Right - missed batch id as one of log params

brmeyer · 2023-05-05T02:30:28Z

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java

+        if (batchStatus.errored_operations > 0) {
+          log.warn("Errored operations count: {}", batchStatus.errored_operations);
+        }
+        //TODO: decide if we need to download/parse batch operations (error responses have only generic info)


@VSydor How hard would it be to do this? Not a huge deal, but it's sometimes helpful to see what specifically failed. Normally it's something like an invalid email address, but at times there is context helpful for debugging...

Already done, actually: added logging for error responses.
However, error response has only generic info:
{ "type": "https://mailchimp.com/developer/marketing/docs/errors/", "title": "Invalid Resource", "status": 400, "detail": "Please provide a valid email address.", "instance": "" }
and does not contain email reference or contact id.

Logging eror responses as:
Failed Batch Operation (status: detail): 400: Please provide a valid email address.

brmeyer · 2023-05-05T02:33:23Z

@VSydor A few last comments, but functionally, it's working great!!!

VSydor requested a review from brmeyer April 19, 2023 15:55

VSydor force-pushed the feature/mailchimp-batch-upsert branch 3 times, most recently from 7b5250e to 5c540d1 Compare April 19, 2023 17:21

brmeyer reviewed Apr 28, 2023

View reviewed changes

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java Outdated Show resolved Hide resolved

brmeyer reviewed Apr 28, 2023

View reviewed changes

src/main/java/com/impactupgrade/nucleus/service/segment/MailchimpEmailService.java Outdated Show resolved Hide resolved

brmeyer reviewed Apr 28, 2023

View reviewed changes

src/main/java/com/impactupgrade/nucleus/client/MailchimpClient.java Show resolved Hide resolved

VSydor force-pushed the feature/mailchimp-batch-upsert branch 6 times, most recently from 2c730ca to 03498d5 Compare May 2, 2023 13:56

VSydor requested a review from brmeyer May 2, 2023 14:00

brmeyer reviewed May 5, 2023

View reviewed changes

VSydor force-pushed the feature/mailchimp-batch-upsert branch 4 times, most recently from b93afe7 to f597adb Compare May 10, 2023 09:58

VSydor requested a review from brmeyer May 10, 2023 09:59

brmeyer force-pushed the feature/mailchimp-batch-upsert branch 2 times, most recently from eeb40df to 8d05bf4 Compare June 19, 2023 16:26

Mailchimp upsert contacts/update tags in batch

d174b0f

brmeyer force-pushed the feature/mailchimp-batch-upsert branch from 8d05bf4 to d174b0f Compare July 12, 2023 21:32

brmeyer merged commit f355223 into master Jul 13, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mailchimp upsert contacts/update tags in batch #124

Mailchimp upsert contacts/update tags in batch #124

VSydor commented Apr 19, 2023

brmeyer Apr 28, 2023

VSydor May 1, 2023

brmeyer May 1, 2023

VSydor May 1, 2023

VSydor May 1, 2023

VSydor May 2, 2023

brmeyer Apr 28, 2023

VSydor May 1, 2023 •

edited

Loading

brmeyer May 1, 2023

VSydor May 1, 2023

VSydor May 2, 2023

brmeyer Apr 28, 2023

VSydor May 1, 2023

brmeyer commented Apr 28, 2023

brmeyer commented May 5, 2023

brmeyer May 5, 2023

VSydor May 10, 2023

brmeyer May 5, 2023

VSydor May 10, 2023 •

edited

Loading

brmeyer commented May 5, 2023

		@@ -32,6 +47,10 @@ public class MailchimpEmailService extends AbstractEmailService {

		private static final Logger log = LogManager.getLogger(MailchimpEmailService.class);

		private static final Integer BATCH_REQUEST_OPERATIONS_SIZE = 50;

Mailchimp upsert contacts/update tags in batch #124

Mailchimp upsert contacts/update tags in batch #124

Conversation

VSydor commented Apr 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VSydor May 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brmeyer commented Apr 28, 2023

brmeyer commented May 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VSydor May 10, 2023 • edited Loading

Choose a reason for hiding this comment

brmeyer commented May 5, 2023

VSydor May 1, 2023 •

edited

Loading

VSydor May 10, 2023 •

edited

Loading