Skip to content

Conversation

vpai
Copy link
Contributor

@vpai vpai commented Sep 15, 2022

Fixes Eppo-exp/eppo#4711

Description

Updated the getAssignment logic to use the allocation (variations + exposure) for the matched targeting rule. The API response inserts a default rule that always matches if the experiment does not have any rules.

See the above design doc for details.

Also made some changes to code organization:

  • Suffixed all dto filenames with -dto and grouped them in a dto folder.
  • Moved client to a new folder client.

How has this been tested?

  • Updated the e2e tests to use new test data for the V2 RAC response.
  • Tested using the SDK from sample code

@vpai vpai requested review from ploomiz and petzel September 15, 2022 00:26
rm -rf $(testDataDir)
mkdir -p $(testDataDir)
gsutil cp gs://sdk-test-data/rac-experiments.json $(testDataDir)
gsutil cp gs://sdk-test-data/rac-experiments-v2.json $(testDataDir)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uses the new test RAC uploaded to S3.

percentExposure: 1,
enabled: true,
subjectShards: 100,
variations: mockVariations,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

variations and percentExposure no longer live at the top level, they live on individual allocations.

end: 67,
const experimentName = 'mock-experiment';

const mockExperimentConfig = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to DRY up this file a bit.

also add comments to getAssignment function
@vpai vpai force-pushed the varun/rac-v2-updates branch from 078b3c1 to a482d5b Compare September 15, 2022 00:29
Copy link
Contributor

@ploomiz ploomiz left a comment

Choose a reason for hiding this comment

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

Looks good, only minor comments

- matchesAnyRule -> findMatchingRule
- remove redundant rule length check
ploomiz and others added 2 commits September 19, 2022 08:49
Co-authored-by: Eric Petzel <epetzel@gmail.com>
@ploomiz ploomiz merged commit c480ca3 into main Sep 19, 2022
@ploomiz ploomiz deleted the varun/rac-v2-updates branch September 19, 2022 16:56
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.

3 participants