feat(marketo_bulk_upload): add support to perform marketo bulk upload polling and fetch job status #4939
Conversation
…, polling and fetch job status
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4939 +/- ##
==========================================
- Coverage 74.44% 74.23% -0.21%
==========================================
Files 428 431 +3
Lines 49763 49989 +226
==========================================
+ Hits 37044 37108 +64
- Misses 10284 10446 +162
Partials 2435 2435 ☔ View full report in Codecov by Sentry. |
|
|
||
| func getJobsStatus(event MarketoAsyncFailedPayload, jobType string, accessToken string) (string, error) { | ||
| config := event.Config | ||
| munchkinId := config["MunchkinId"] |
There was a problem hiding this comment.
Do we need to do a nil check for config?
| endTime := time.Now() | ||
| requestTime := endTime.Sub(startTime).Milliseconds() | ||
|
|
||
| fmt.Printf("Request time: %d ms\n", requestTime) |
|
|
||
| data := make(map[int64]string) | ||
|
|
||
| fieldSchemaMapping, err := getFieldSchemaMap(accessToken, config["MunchkinId"]) |
There was a problem hiding this comment.
can we use some CSV reader rather than doing manually
| elemArr = elemArr[:len(elemArr)-1] | ||
|
|
||
| for key, val := range data { | ||
| if val == strings.Join(elemArr, ",") { |
There was a problem hiding this comment.
do we need to join for every comparison? can we do this outside the loop?
| elemArr = elemArr[:len(elemArr)-1] | ||
|
|
||
| for key, val := range data { | ||
| if val == strings.Join(elemArr, ",") { |
There was a problem hiding this comment.
we should use the lookup field for comparison not compare the whole record
https://developer.adobe.com/marketo-apis/api/mapi/#tag/Bulk-Import-Leads
There was a problem hiding this comment.
There can be one possible change in the functionality. The user needs to ensure that lookup field should be chosen as primary key while configuring the source.
Here is the reason:
Let's say lookup field is Email, and while configuring the source if we choose ID as a primary key, then source can query the rows where id must be present but email can be null / empty string etc.
In that case, Marketo throws some warning.
I was sending the below data in csv:
FirstName,LastName,Email,Company
'Able','Baker','', 'marketo'
Able,"Baker","dummy@gmail.com", "marketo"
And while polling I got response :
{
"requestId": "14cf8#1912627bf66",
"result": [
{
"batchId": 7271,
"importId": "7271",
"status": "Complete",
"numOfLeadsProcessed": 2,
"numOfRowsFailed": 0,
"numOfRowsWithWarning": 1,
"message": "Import succeeded, 2 records imported (2 members), 1 warning."
}
],
"success": true
}
For this the code flow will appear in fetch job status part, and if we search with only lookup field, we will not get anything to refer from. Previously we were marking these kind of warnings as failure, but here, it will be automatically marked as success.
The response :
FirstName,LastName,Email,Company,Import Warning Reason
'Able','Baker','', 'marketo',Invalid email address
@koladilip let me know if you need some more clarification.
There was a problem hiding this comment.
this is a rare case and it is fine, we can update a footer note in the form, I don't think this breaks our logic so we are good
…, polling and fetch job status
|
This PR is considered to be stale. It has been open 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
Description
< Replace with adequate description for this PR as per Pull Request document >
Linear Ticket
< Replace with Linear Link ( create or search linear ticket) or >
Security