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

fix: json marshal l errors when parsing poll response from klaviyo #5316

Merged
merged 11 commits into from
Dec 9, 2024

Conversation

koladilip
Copy link
Contributor

Description

There is a bug in the Klaviyo implementation where, at times, we receive an empty import ID from the upload. However, when we poll with an empty import ID, we encounter parsing errors because Klaviyo returns a list response when the import ID is empty.

Linear Ticket

https://linear.app/rudderstack/issue/INT-2820/klaviyo-bulk-upload-at-times-they-are-running-into-errors
Resolves INT-2820

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

achettyiitr and others added 7 commits November 14, 2024 10:59
* fix: bing ads audience (#5299)

* chore: handle 500kb limit, refactor to service impl (#5302)

* chore: handle 500kb limit, refactor to service impl

* chore: fix mock

* chore: fix mock newline

* chore: fix apiservice newline

* chore: move error

---------

Co-authored-by: Sudip Paul <67197965+ItsSudip@users.noreply.github.com>
Copy link

codecov bot commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 51.85185% with 13 lines in your changes missing coverage. Please review.

Project coverage is 74.71%. Comparing base (dc446ee) to head (002defa).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...destinationmanager/klaviyobulkupload/apiService.go 42.85% 12 Missing ⚠️
...tionmanager/klaviyobulkupload/klaviyobulkupload.go 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5316      +/-   ##
==========================================
- Coverage   74.80%   74.71%   -0.10%     
==========================================
  Files         437      437              
  Lines       61211    61230      +19     
==========================================
- Hits        45788    45747      -41     
- Misses      12900    12947      +47     
- Partials     2523     2536      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yashasvibajpai yashasvibajpai changed the base branch from release/1.38.x to master December 9, 2024 05:55
@yashasvibajpai yashasvibajpai changed the base branch from master to release/1.38.x December 9, 2024 05:56
…destination (#5353)

* fix: allow only enabled dest in backendSubscriber, fix klaviyo bulk

* chore: add rate limited http client for klaviyo bulk

* refactor: add rate limiter klaviyo bulk upload

---------

Co-authored-by: Dilip Kola <kdilipkola@gmail.com>
@yashasvibajpai yashasvibajpai changed the base branch from release/1.38.x to master December 9, 2024 06:09
@yashasvibajpai yashasvibajpai changed the base branch from master to release/1.38.x December 9, 2024 06:10
@koladilip koladilip changed the base branch from release/1.38.x to master December 9, 2024 06:20
@koladilip koladilip requested a review from achettyiitr December 9, 2024 06:59
@koladilip koladilip merged commit 446666b into master Dec 9, 2024
58 checks passed
@koladilip koladilip deleted the fix.klaviyo-poll-json-marshall-issues branch December 9, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants