Skip to content

Support SSRC conditions #2487

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

Merged
merged 47 commits into from
Mar 21, 2024
Merged

Support SSRC conditions #2487

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
81c90c3
Get And condition passing
erikeldridge Feb 29, 2024
fc7deeb
Support and.or condition
erikeldridge Feb 29, 2024
bd12f0d
Support true condition
erikeldridge Feb 29, 2024
43e479e
Support false condition, and fix tests
erikeldridge Feb 29, 2024
64c71d7
Use or.and, not the other way around
erikeldridge Feb 29, 2024
dd3af1d
Integrate conditional values into evaluate method
erikeldridge Mar 5, 2024
14f5339
Test handling for multiple conditions
erikeldridge Mar 5, 2024
1754fb5
Clean up logs
erikeldridge Mar 5, 2024
0283dbf
Extract condition evaluation to class for testing
erikeldridge Mar 5, 2024
264c75f
Namespace condition names
erikeldridge Mar 5, 2024
fcbcd6d
Iterate over ordered condition list
erikeldridge Mar 5, 2024
ef07c33
Test condition ordering
erikeldridge Mar 5, 2024
9179a0f
Differentiate named conditions
erikeldridge Mar 5, 2024
816f38a
Document condition types
erikeldridge Mar 5, 2024
307ec9d
Generalize condition eval test and fix styling
erikeldridge Mar 5, 2024
f87e605
Replace log statement with todo
erikeldridge Mar 5, 2024
4599ec5
Implement evaluate percent condition for RC server-side
trekforever Mar 6, 2024
559d8aa
Apply lint fixes
trekforever Mar 6, 2024
31d6c9a
Add context param to evaluate method
erikeldridge Mar 7, 2024
4e6a1c9
Add tests for percent condition eval
trekforever Mar 8, 2024
6e86cba
Update evaluator tests to use context
erikeldridge Mar 8, 2024
b669472
Increase threshold to +/- 500 for percent condition eval tests
trekforever Mar 8, 2024
687daa3
Clean up percentCondition tests a bit and add note on the tolerance used
trekforever Mar 8, 2024
12101f1
Apply suggestions from code review
erikeldridge Mar 9, 2024
89efaa7
Update copyright date and remove stray log statement
erikeldridge Mar 9, 2024
d161bd6
Mock farmhash in tests
erikeldridge Mar 11, 2024
9e0bc5d
Add Math.abs for farmhash - to be consistent with the internal implem…
trekforever Mar 12, 2024
bdc14d3
Regenerate package-lock to fix Node 14 CI error re busboy
erikeldridge Mar 13, 2024
0a3408f
Fix lint errors
erikeldridge Mar 13, 2024
347f07b
Rename "id" to "randomizationId" per discussion
erikeldridge Mar 14, 2024
6e245e2
Extract API
erikeldridge Mar 14, 2024
f7dbf1d
Only return false in cases of uknown template evaluation
erikeldridge Mar 14, 2024
0cab5d1
Remove product prefix from type names
erikeldridge Mar 14, 2024
53c2145
Remove product prefix from exported types
erikeldridge Mar 15, 2024
ee675fb
Remove unused "expression" field from server condition
erikeldridge Mar 15, 2024
c9701eb
Extract API
erikeldridge Mar 15, 2024
2f2fa2f
Merge branch 'ssrc-prefix' into ssrc-percent
erikeldridge Mar 15, 2024
fa21a91
Remove prefix from impl classes, for consistency
erikeldridge Mar 15, 2024
caddabe
Merge branch 'ssrc-prefix' into ssrc-percent
erikeldridge Mar 15, 2024
530ae21
Remove prefix from new internal classes
erikeldridge Mar 15, 2024
97876df
Remove "server" prefix
erikeldridge Mar 19, 2024
689c6aa
Remove prefix from NamedCondition
erikeldridge Mar 19, 2024
9c79eea
Merge branch 'ssrc-prefix' into ssrc-percent
erikeldridge Mar 19, 2024
0202ac7
Rename "or" and "and" fields to match API
erikeldridge Mar 20, 2024
14fdd8e
Rename "operator" field to "percentOperator" to match API
erikeldridge Mar 20, 2024
117eae9
Extract API after "and" and "or" rename
erikeldridge Mar 20, 2024
e99c489
Merge remote-tracking branch 'gh-public/ssrc' into ssrc-percent
erikeldridge Mar 21, 2024
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Apply suggestions from code review 
Co-authored-by: jen_h <harveyjen@google.com>
  • Loading branch information
@erikeldridge @jenh
erikeldridge and jenh authored Mar 9, 2024
commit 12101f1fd097973df7e06c7125aa4c69c1c538f9
2 changes: 1 addition & 1 deletion src/remote-config/remote-config-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export interface RemoteConfigCondition {
/**
* Represents a Remote Config condition in the dataplane.
* A condition targets a specific group of users. A list of these conditions
* make up part of a Remote Config template.
* comprise part of a Remote Config template.
*/
export interface RemoteConfigServerNamedCondition {
Copy link
Contributor Author

@erikeldridge erikeldridge Mar 8, 2024

Choose a reason for hiding this comment

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

@lahirumaramba I see some interfaces with the RemoteConfig prefix and others without. Is there a guideline for what needs the prefix?
Copy link
Member

@lahirumaramba lahirumaramba Mar 13, 2024

Choose a reason for hiding this comment

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

I can't recall a reason. Looking at this proposal (go/admin-sdk-remote-config) from 2020 it looks like the types that were added later are missing the prefix.... maybe we weren't strict about the prefix in follow up proposals?
Copy link
Contributor Author

@erikeldridge erikeldridge Mar 19, 2024

Choose a reason for hiding this comment

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

I was just reviewing a change for the REST API and I think what happened is the TS API naming biased toward the REST API. The REST API has nested protos, but TS interfaces can't be nested. The parent protos had more qualified names than the child protos. When we created interfaces to match the protos, we retained the existing names.

I think the guideline we discussed against product prefixes still holds and will help the TS API be more consistent.

Expand Down
4 changes: 2 additions & 2 deletions test/unit/remote-config/remote-config-condition-evaluator.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,8 @@ describe('RemoteConfigConditionEvaluator', () => {
expect(truthyAssignments).to.be.lessThanOrEqual(50000 + range);
});

// Returns the number of assignments which evaluate to true for the specified percent condition
// This method randomly generates the ids for each assignment for this purpose
// Returns the number of assignments which evaluate to true for the specified percent condition.
// This method randomly generates the ids for each assignment for this purpose.
function evaluateRandomAssignments(
condition: RemoteConfigServerPercentCondition,
numOfAssignments: number,
Expand Down