feat: create EppoPrecomputedClient #129
Conversation
* Add methods for boolean, integer, numeric, and json types * Add tests for typed assignments * Remove eslint warnings
package.json
Outdated
| { | ||
| "name": "@eppo/js-client-sdk-common", | ||
| "version": "4.3.0", | ||
| "version": "4.3.1-alpha.0", |
There was a problem hiding this comment.
I will bump this pre-release version with the following changes:
- export the
PrecomputedFlaginterface for setting up the memory only store's type - update the
PRECOMPUTED_BASE_URLandPRECOMPUTED_FLAGS_ENDPOINT
| export const PRECOMPUTED_BASE_URL = 'https://fs-edge-assignment.eppo.cloud'; | ||
| export const PRECOMPUTED_FLAGS_ENDPOINT = '/assignments'; |
There was a problem hiding this comment.
Bingo - Eric P. and I have a plan for migrating this to https://fscdn.eppo.cloud but will do it after the holiday.
BTW do you think I should version it?
There was a problem hiding this comment.
Not necessary here, but yes it would be consistent with the UFC endpoint if it is versioned
| PRECOMPUTED = 'PRECOMPUTED', | ||
| } | ||
|
|
||
| export interface PrecomputedFlag { |
|
|
||
| /** | ||
| * The flag evaluation details | ||
| * The flag evaluation details. Null if the flag was precomputed. |
There was a problem hiding this comment.
Is it also null if users don't request details?
There was a problem hiding this comment.
With this implementation it is always null. The edge compute endpoint would need to provide these details in the response for it to be added here, which is out of scope for this milestone
| import { Attributes } from './types'; | ||
|
|
||
| export interface FlagEvaluation { | ||
| export interface FlagEvaluationWithoutDetails { |
| const precomputedFlag: PrecomputedFlag | null = this.precomputedFlagStore.get( | ||
| getMD5Hash(flagKey), | ||
| ) as PrecomputedFlag; | ||
| return precomputedFlag ? decodePrecomputedFlag(precomputedFlag) : null; |
There was a problem hiding this comment.
The server is NOT performing obfuscation in the current implementation - this might have gotten muddled in the design doc.
There was a problem hiding this comment.
Yeah I added support for it in the SDK, but it's not necessary for the server to be obfuscating yet
There was a problem hiding this comment.
We'll need obfuscation eventually, but it's okay to leave it out of the first milestone
| doLog: true, | ||
| }, | ||
| }, | ||
| }; // TODO: readMockPrecomputedFlagsResponse(MOCK_PRECOMPUTED_FLAGS_RESPONSE_FILE); |
| }); | ||
| }); | ||
|
|
||
| it('returns null if getStringAssignment was called for the subject before any precomputed flags were loaded', () => { |
| expect(td.explain(mockLogger.logAssignment).callCount).toEqual(1); | ||
| }); | ||
|
|
||
| it('logs assignment again after the lru cache is full', async () => { |
There was a problem hiding this comment.
I'm glad you are thinking about testing the assignment logging and deduplication - as you mentioned there is some duplication here. Could you consider a way in the future to have the two eppo clients share these?
src/http-client.ts
Outdated
|
|
||
| async getPrecomputedFlags(): Promise<IPrecomputedFlagsResponse | undefined> { | ||
| const url = this.apiEndpoints.precomputedFlagsEndpoint(); | ||
| return await this.rawGet<IPrecomputedFlagsResponse>(url); |
There was a problem hiding this comment.
🟡 Quick thing I just noticed - the endpoint is a POST
There was a problem hiding this comment.
Yep, I bumped the prerelease version (https://www.npmjs.com/package/@eppo/js-client-sdk-common/v/4.3.1-alpha.2) with this change #137 and the requests work now when I run the client in a script. It'll work in the browser after a CORS fix
* Add method for post * Change request to a post * Fix payload format * v4.3.1-alpha.2
|
@sameerank this PR could really use some fleshing out of the motivation and description. even if after merged, that'd still be helpful and appreciated |
labels: mergeable
Fixes: #issue
Motivation and Context
An edge compute function will generate and return a JSON of precomputed assignments for each flag. This created a new client to handle this format. The benefits of precomputing assignments is 1) smaller payloads and 2) greater security, because targeting rules with potentially sensitive information would no longer need to be sent over a public network.
Description
The
EppoPrecomputedClientis a separate singleton fromEppoClientso that we can run both simultaneously for dogfooding. In order to do this,EppoPrecomputedClientcomes with its own distinct "configuration store" and precomputed flags requestor, so that these wouldn't overlap with theEppoClientinstance. I also wrote tests to verify the core functionality -- most are similar to theEppoClienttests but modified slightly to be compatible with theEppoPrecomputedClientformat.How has this been tested?