-
Notifications
You must be signed in to change notification settings - Fork 2
Offline Init Part 1 of 4: Add getFlagsConfiguration() to export flag configuration as JSON #116
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
base: main
Are you sure you want to change the base?
Changes from all commits
be8cc13
e6fcbfd
1b804de
d777a2c
5136bc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,7 +32,14 @@ import { | |||||||||
|
|
||||||||||
| import * as util from './util/index'; | ||||||||||
|
|
||||||||||
| import { getInstance, IAssignmentEvent, IAssignmentLogger, init, NO_OP_EVENT_DISPATCHER } from '.'; | ||||||||||
| import { | ||||||||||
| getFlagsConfiguration, | ||||||||||
| getInstance, | ||||||||||
| IAssignmentEvent, | ||||||||||
| IAssignmentLogger, | ||||||||||
| init, | ||||||||||
| NO_OP_EVENT_DISPATCHER, | ||||||||||
| } from '.'; | ||||||||||
|
|
||||||||||
| import SpyInstance = jest.SpyInstance; | ||||||||||
|
|
||||||||||
|
|
@@ -501,6 +508,10 @@ describe('EppoClient E2E test', () => { | |||||||||
| }, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| afterEach(() => { | ||||||||||
| td.reset(); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| it('retries initial configuration request before resolving', async () => { | ||||||||||
| td.replace(HttpClient.prototype, 'getUniversalFlagConfiguration'); | ||||||||||
| let callCount = 0; | ||||||||||
|
|
@@ -635,8 +646,6 @@ describe('EppoClient E2E test', () => { | |||||||||
| let isReadOnlyFsSpy: SpyInstance; | ||||||||||
|
|
||||||||||
| beforeEach(() => { | ||||||||||
|
||||||||||
| beforeEach(() => { | |
| beforeEach(() => { | |
| // Reset the module before each test to avoid shared module-level state between tests | |
| jest.resetModules(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,14 @@ export { IClientConfig }; | |
|
|
||
| let clientInstance: EppoClient; | ||
|
|
||
| // We keep references to the configuration stores at module level because EppoClient | ||
| // does not expose public getters for store metadata (format, createdAt, environment) | ||
| // or bandit configurations. These references are needed by getFlagsConfiguration() | ||
| // and getBanditsConfiguration() to reconstruct exportable configuration JSON. | ||
| let flagConfigurationStore: MemoryOnlyConfigurationStore<Flag>; | ||
| let banditVariationConfigurationStore: MemoryOnlyConfigurationStore<BanditVariation[]>; | ||
| let banditModelConfigurationStore: MemoryOnlyConfigurationStore<BanditParameters>; | ||
|
Comment on lines
+44
to
+50
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trying not to mess with the core SDK for now, but future us can expose these which would tidy this up a bit. |
||
|
|
||
| export const NO_OP_EVENT_DISPATCHER: EventDispatcher = { | ||
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| attachContext: () => {}, | ||
|
|
@@ -86,9 +94,9 @@ export async function init(config: IClientConfig): Promise<EppoClient> { | |
| throwOnFailedInitialization, | ||
| }; | ||
|
|
||
| const flagConfigurationStore = new MemoryOnlyConfigurationStore<Flag>(); | ||
| const banditVariationConfigurationStore = new MemoryOnlyConfigurationStore<BanditVariation[]>(); | ||
| const banditModelConfigurationStore = new MemoryOnlyConfigurationStore<BanditParameters>(); | ||
| flagConfigurationStore = new MemoryOnlyConfigurationStore<Flag>(); | ||
| banditVariationConfigurationStore = new MemoryOnlyConfigurationStore<BanditVariation[]>(); | ||
| banditModelConfigurationStore = new MemoryOnlyConfigurationStore<BanditParameters>(); | ||
| const eventDispatcher = newEventDispatcher(apiKey, eventTracking); | ||
|
|
||
| clientInstance = new EppoClient({ | ||
|
|
@@ -144,6 +152,107 @@ export function getInstance(): EppoClient { | |
| return clientInstance; | ||
| } | ||
|
|
||
| /** | ||
| * Reconstructs the current flags configuration as a JSON string. | ||
| * This can be used to bootstrap another SDK instance using offlineInit(). | ||
| * | ||
| * @returns JSON string containing the flags configuration, or null if not initialized | ||
| * @public | ||
| */ | ||
| export function getFlagsConfiguration(): string | null { | ||
| if (!flagConfigurationStore) { | ||
| return null; | ||
| } | ||
|
|
||
| const flags = flagConfigurationStore.entries(); | ||
| const format = flagConfigurationStore.getFormat(); | ||
| const createdAt = flagConfigurationStore.getConfigPublishedAt(); | ||
| const environment = flagConfigurationStore.getEnvironment(); | ||
|
|
||
| const configuration: { | ||
| createdAt?: string; | ||
| format?: string; | ||
| environment?: { name: string }; | ||
| flags: Record<string, Flag>; | ||
| banditReferences?: Record< | ||
| string, | ||
| { | ||
| modelVersion: string; | ||
| flagVariations: BanditVariation[]; | ||
| } | ||
| >; | ||
| } = { | ||
| flags, | ||
| }; | ||
|
|
||
| if (createdAt) { | ||
| configuration.createdAt = createdAt; | ||
| } | ||
| if (format) { | ||
| configuration.format = format; | ||
| } | ||
| if (environment) { | ||
| configuration.environment = environment; | ||
| } | ||
|
Comment on lines
+188
to
+196
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If some of these values were null or undefined, it would be an invalid configuration. It might be better to return defaults instead. This would also allow you to reuse the So you could do something like this:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also related to this comment |
||
|
|
||
| const banditReferences = reconstructBanditReferences(); | ||
| if (banditReferences) { | ||
| configuration.banditReferences = banditReferences; | ||
| } | ||
|
|
||
| return JSON.stringify(configuration); | ||
| } | ||
|
|
||
| /** | ||
| * Reconstructs banditReferences from stored variations and parameters. | ||
| * The variations are stored indexed by flag key, so we need to re-pivot them | ||
| * back to being indexed by bandit key for export. | ||
| */ | ||
| function reconstructBanditReferences(): Record< | ||
| string, | ||
| { modelVersion: string; flagVariations: BanditVariation[] } | ||
| > | null { | ||
| if (!banditVariationConfigurationStore || !banditModelConfigurationStore) { | ||
| return null; | ||
| } | ||
|
|
||
| const variationsByFlagKey = banditVariationConfigurationStore.entries(); | ||
| const banditParameters = banditModelConfigurationStore.entries(); | ||
|
|
||
| // Flatten all variations and group by bandit key | ||
| const variationsByBanditKey: Record<string, BanditVariation[]> = {}; | ||
| for (const variations of Object.values(variationsByFlagKey)) { | ||
| for (const variation of variations) { | ||
| const banditKey = variation.key; | ||
| if (!variationsByBanditKey[banditKey]) { | ||
| variationsByBanditKey[banditKey] = []; | ||
| } | ||
| variationsByBanditKey[banditKey].push(variation); | ||
| } | ||
| } | ||
|
|
||
| // Build banditReferences with model versions | ||
| const banditReferences: Record< | ||
| string, | ||
| { modelVersion: string; flagVariations: BanditVariation[] } | ||
| > = {}; | ||
| for (const [banditKey, variations] of Object.entries(variationsByBanditKey)) { | ||
| const params = banditParameters[banditKey]; | ||
| if (params) { | ||
| banditReferences[banditKey] = { | ||
| modelVersion: params.modelVersion, | ||
| flagVariations: variations, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(banditReferences).length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return banditReferences; | ||
| } | ||
|
|
||
| function newEventDispatcher( | ||
| sdkKey: string, | ||
| config: IClientConfig['eventTracking'] = {}, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
afterEachblock callstd.reset()but the corresponding test 'retries initial configuration request before resolving' usestd.replace()on HttpClient.prototype. This cleanup should be in the existing test's cleanup (or a dedicated beforeEach/afterEach within that specific test), not added at the describe block level where it affects all tests in the 'Configuration Request' suite.