-
Notifications
You must be signed in to change notification settings - Fork 299
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
feat: bing ads bulk upload #3371
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
don't merge to master yet |
koladilip
reviewed
May 25, 2023
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/marketo-bulk-upload/marketobulkupload.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/marketo-bulk-upload/marketobulkupload.go
Outdated
Show resolved
Hide resolved
koladilip
reviewed
May 25, 2023
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
koladilip
reviewed
May 25, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
) * fix: adding temporary commits * feat: initial commit for poll function * fix: removing commented out code * fix: small update in poll function * feat: fetch failed results for bingAds * fix: small edit in function definition * fix: shortening function parameters fot fetchFailedEvents * update message structure of bingads_audience (#3444) * fix: adding success key logic and addressing review comments * fix: making poll url an optional field * fix: fixing in the status code data type * fix: fixing rudder-auth service connection and editing test cases(#3455) * update message structure of bingads_audience * add implementation of tokenSource interface * update existing testcases * fix: fixing marketo bulk upload struct * fix: small comments added * fix: removing the poll status file after the response is written * fix: adding function comments * fix: remove the zip file after data upload --------- Co-authored-by: Sudip Paul <67197965+ItsSudip@users.noreply.github.com>
lvrach
reviewed
Jun 8, 2023
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/asyncdestinationmanager.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/marketo-bulk-upload/marketobulkupload.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Leonidas Vrachnis <leo.al.vra@gmail.com>
Co-authored-by: Leonidas Vrachnis <leo.al.vra@gmail.com>
Co-authored-by: Leonidas Vrachnis <leo.al.vra@gmail.com>
Co-authored-by: Leonidas Vrachnis <leo.al.vra@gmail.com>
Co-authored-by: Leonidas Vrachnis <leo.al.vra@gmail.com>
cisse21
approved these changes
Jul 27, 2023
koladilip
reviewed
Jul 27, 2023
router/batchrouter/asyncdestinationmanager/bing-ads/bulk_uploader.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/bing-ads/bulk_uploader.go
Outdated
Show resolved
Hide resolved
router/batchrouter/asyncdestinationmanager/bing-ads/bulk_uploader.go
Outdated
Show resolved
Hide resolved
…der.go Co-authored-by: Dilip Kola <33080863+koladilip@users.noreply.github.com>
koladilip
approved these changes
Aug 1, 2023
* fix: initial commit * chore: clean up * chore: clean up * chore: clean up * chore: clean up --------- Co-authored-by: shrouti1507 <shrouti.gangopadhyay@gmail.com> Co-authored-by: Chandra shekar Varkala <chandra@rudderlabs.com>
chandumlg
approved these changes
Aug 9, 2023
This was referenced Aug 9, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
resolves INT-244
We are currently undergoing a revamp of the batch router logic to accommodate asynchronous destinations. In the previous version, we had support for a single asynchronous destination, which was Marketo bulk upload. However, we are now expanding our capabilities to include Bing ads audience as well. To ensure data is sent correctly to these destinations, we have identified the following two crucial steps:
Transformation and Upload:
The first step involves transforming the text data into the required format and subsequently uploading the transformed file to the respective destination. Previously, this entire process was handled by the transformer. However, considering the potential inefficiency of sending large files to the transformer repeatedly, we have decided to execute these steps on the server itself.Status Monitoring:
The second step is to fetch the status of the uploaded file from the destination. This allows us to determine the success or failure of the upload. In the event of a failure, it is important to retrieve the specific reason for the failure, enabling effective troubleshooting and issue resolution.To facilitate these revised processes, we have introduced a new interface called AsyncDestinationManager. This interface comprises the following essential functions:
Upload:
This function takes care of transforming the file into the required format and performing the upload process to the destination. By handling this process on the server, we can optimize efficiency and avoid unnecessary data transfers.Poll:
The poll function is responsible for regularly checking and fetching the status of the uploaded file from the destination. This enables us to monitor the progress and ensure timely and accurate data transmission.FetchFailedEvents:
In the case of any failed events during the upload process, the FetchFailedEvents function allows us to retrieve the specific details of the failed event. This information is crucial for troubleshooting and taking appropriate corrective actions.By introducing the AsyncDestinationManager interface and implementing these functions, we are streamlining the batch router logic to efficiently handle asynchronous destinations. This approach minimizes data transfer overhead and improves overall performance while providing comprehensive monitoring and error-handling capabilities.
Notion Ticket
https://www.notion.so/rudderstacks/d5d15ce4be354e3caae11d4ea9f41477?v=7e2d9df8de2f4251a8a80a4cf2c3f662&p=a3e076e645f0474fbd18bd397f982d14&pm=c
Security