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.

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: add local override functionality #184

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

feat: add local override functionality #184

wants to merge 17 commits into from

Conversation

yfrancis
Copy link
Contributor

If overrides have been supplied, use those instead of evaluating rules. Leverages a user-supplied configuration store if present, which may be backed by memory or local storage.

Copy link
Member

@leoromanovsky leoromanovsky left a comment

Choose a reason for hiding this comment

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

The obfuscation support for this feature would benefit from a design doc with additional eyes on it; it would be a good reason to evolve our security posture for example beyond md5 (a deprecated algorithm).

src/client/eppo-client.ts Show resolved Hide resolved
src/client/eppo-client.ts Outdated Show resolved Hide resolved
@yfrancis yfrancis changed the title Add local override functionality feat: add local override functionality Dec 18, 2024
Copy link
Contributor

@aarsilv aarsilv left a comment

Choose a reason for hiding this comment

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

Looks like a good foundation for overrides, and I think getAssignmentDetail() is the right place to check for an override.

We will need to be sure to check in the right place for the precomputed client as well.

I think overrides like this could be useful for testing and simple, client-only deployments. For customers using server flags as well, I think we'll need to on-the-fly adjust targeting rules or something to that effect. A mini design review could be helpful in laying out the various use cases.

Note that if we do release this, we'll want to bump the version in package.json before releasing. This should be developed concurrently with the upstream SDK(s) as well.

src/client/eppo-client.spec.ts Show resolved Hide resolved
src/client/eppo-client.spec.ts Show resolved Hide resolved
src/client/eppo-client.ts Outdated Show resolved Hide resolved
src/client/eppo-client.ts Show resolved Hide resolved
src/client/eppo-client.ts Show resolved Hide resolved
src/client/eppo-client.ts Outdated Show resolved Hide resolved
src/client/eppo-client.ts Outdated Show resolved Hide resolved
});

it('returns override values for all supported types', () => {
overrideStore.setEntries({
Copy link
Contributor

Choose a reason for hiding this comment

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

Upstream overrideStore is not exposed, so I imagine we need a setOverride() method on the client to add an entry to this store, if present.

src/client/eppo-client.ts Outdated Show resolved Hide resolved
src/client/eppo-client.ts Show resolved Hide resolved
src/client/eppo-client.ts Outdated Show resolved Hide resolved
@yfrancis yfrancis force-pushed the yf-overrides branch 2 times, most recently from 7d01d0f to 0fee52a Compare January 29, 2025 23:08
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.

4 participants