channels-1094 #2024
channels-1094 #2024
Conversation
engageDestinationCache. This is because this cache is no longer just intended to be used for DataFeed cache, but is now intended to be used as a more generic cache for the Engage Destination.
| /** | ||
| * The features available in the request based on the customer's sourceID; | ||
| * `features`, `stats`, `logger`, `dataFeedCache`, and `transactionContext` and `stateContext` are for internal Twilio/Segment use only. | ||
| */ | ||
| /** Engage internal use only. DO NOT USE. */ |
There was a problem hiding this comment.
Broke this comment apart so that we could get language server support for using each property in the shape of this object.
| /** | ||
| * The features available in the request based on the customer's sourceID; | ||
| * `features`,`stats`, `logger` , `transactionContext` and `stateContext` are for internal Twilio/Segment use only. | ||
| */ | ||
| /** Engage internal use only. DO NOT USE. */ |
There was a problem hiding this comment.
Broke this comment apart so that we could get language server support for using each property in the shape of this object.
packages/destination-actions/src/destinations/engage/utils/MessageSendPerformer.ts
Outdated
Show resolved
Hide resolved
packages/destination-actions/src/destinations/engage/utils/MessageSendPerformer.ts
Outdated
Show resolved
Hide resolved
| } | ||
| // Check if the response is successful. | ||
| if (response.ok) { | ||
| const body = await response.text() |
There was a problem hiding this comment.
I might be wrong but reading response.text() here will close the stream and will fail next time response body is tried to be read (during actual logic of processing response), so you might want to check it and if it's the case you may use response.clone() or pass here the actual value to be cached, instead of response object
There was a problem hiding this comment.
Since I'm swapping to use the status code I don't think we need to worry about this.
packages/destination-actions/src/destinations/engage/sendgrid/sendEmail/SendEmailPerformer.ts
Outdated
Show resolved
Hide resolved
packages/destination-actions/src/destinations/engage/utils/MessageSendPerformer.ts
Outdated
Show resolved
Hide resolved
|
hi @cogwizzle @pmunin please let me know when you are done with this so I can review prior to deploy |
… into channels-1094
… into channels-1094
* fixing stats bug * chore: Update "@segment/actions-shared" dependency to version 1.105.1-channels-1200.59 * fix: Initialize EngageLogger and EngageStats in EngageActionPerformer constructor * fix: common tags * chore: Update "@segment/actions-shared" dependency to version 1.105.1-channels-1200.62 * clean up dups * reverting changes that are not relevant to PR from master * reverting irrelevant changes * reverting irrelevant changes --------- Co-authored-by: Philipp Munin <7120440+pmunin@users.noreply.github.com>
…tinations into channels-1094
…tinations into channels-1094
|
Closing towards #2325 |
refactor: Refactoring the name for dataFeedCache to be
engageDestinationCache. This is because this cache is no longer just intended to be used for DataFeed cache, but is now intended to be used as a more generic cache for the Engage Destination.
A summary of your pull request, including the what change you're making and why.
Testing
The changes should be tested in the PR in integrations.
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.
This change was tested in conjunction with the Channels-1094 branch in integrations. We saw 2000 duplicates prevented during our load test. Additionally we had 624 duplicates that still showed up in Splunk. In prior load test of the same size this number was close to 3000.