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.

Sign up for GitHub

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

Jump to bottom

adding batch support #2192

Merged
merged 8 commits into from
Jul 29, 2024
Merged

adding batch support #2192

merged 8 commits into from
Jul 29, 2024

Conversation

joe-ayoub-segment
Copy link
Contributor

@joe-ayoub-segment joe-ayoub-segment commented Jul 18, 2024 •
edited
Loading

Adding batch capability to Optimizely Audiences.

Testing

Tested locally.

@joe-ayoub-segment joe-ayoub-segment self-assigned this Jul 22, 2024
@joe-ayoub-segment
Copy link
Contributor Author

Hi @sayan-das-in any chance of a review of this please?
I'm specially keen to get some feedback on how the rawData data is being accessed.
The original code did make use of rawData to access the traits or properties objects. However when supporting batches I assume that rawData will be passed as an array. This is how I tested it locally anyway.

@joe-ayoub-segment joe-ayoub-segment marked this pull request as ready for review July 22, 2024 09:21
@joe-ayoub-segment joe-ayoub-segment requested a review from a team as a code owner July 22, 2024 09:21
sayan-das-in
sayan-das-in previously approved these changes Jul 22, 2024
View reviewed changes
@Romani-Ramzi
Copy link
Contributor

@joe-ayoub-segment seems good, tests are updated to test either flow, the batch and the single perform method.
All good from our side

@joe-ayoub-segment
Copy link
Contributor Author

Note: Can't deploy until tested in Staging.

varadarajan-tw
varadarajan-tw reviewed Jul 29, 2024
View reviewed changes
...ns/src/destinations/optimizely-advanced-audience-targeting/syncAudience/optimizely-client.ts
this.request = request
this.settings = data.settings
this.payloads = Array.isArray(data.payload) ? data.payload : [data.payload]
this.propsOrTraits = Array.isArray(data.rawData) ? data.rawData : [data.rawData]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using rawData here?

Copy link
Contributor

@varadarajan-tw varadarajan-tw Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we just add a mapping with $.properties or $.traits and use that instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we just add a mapping with $.properties or $.traits and use that instead.

This would break for existing users/customers.

@joe-ayoub-segment joe-ayoub-segment merged commit 074df25 into segmentio:main Jul 29, 2024
6 of 11 checks passed
@joe-ayoub-segment
Copy link
Contributor Author

hi @Romani-Ramzi this PR has been deployed

@Romani-Ramzi
Copy link
Contributor

will "perform" function still send single data but in array style?
does it mean that we will still keep getting high traffic but its just an array?

marinhero pushed a commit that referenced this pull request Aug 2, 2024
@joe-ayoub-segment @ISapoI
@Romani-Ramzi @marinhero
adding batch support (#2192)
a353c28 
* adding batch support

* Fix Optimizely Unit test after batching

* removing mocked payload

* fixing rawData bug

* removing unnecessary interface

* updated tests

* submitting generated types

---------

Co-authored-by: Sam Pontefract <sam.pontefract@optimizely.com>
Co-authored-by: Romani-Ramzi <romani.ramzi@optimizely.com>
Co-authored-by: Romani Ramzi <102735170+Romani-Ramzi@users.noreply.github.com>
harsh-joshi99 pushed a commit that referenced this pull request Aug 16, 2024
@joe-ayoub-segment @ISapoI
@Romani-Ramzi @harsh-joshi99
adding batch support (#2192)
ef3e761 
* adding batch support

* Fix Optimizely Unit test after batching

* removing mocked payload

* fixing rawData bug

* removing unnecessary interface

* updated tests

* submitting generated types

---------

Co-authored-by: Sam Pontefract <sam.pontefract@optimizely.com>
Co-authored-by: Romani-Ramzi <romani.ramzi@optimizely.com>
Co-authored-by: Romani Ramzi <102735170+Romani-Ramzi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varadarajan-tw varadarajan-tw varadarajan-tw approved these changes

@Romani-Ramzi Romani-Ramzi Romani-Ramzi approved these changes

@sayan-das-in sayan-das-in sayan-das-in left review comments

Assignees

@joe-ayoub-segment joe-ayoub-segment

Labels
deploy:push mode:cloud partner-eng-review (cloud) team:segment-core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@joe-ayoub-segment @Romani-Ramzi @sayan-das-in @varadarajan-tw @ISapoI