-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add support for skipInitialPoll parameter (FF-2084) #64
Conversation
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.
📈
src/index.spec.ts
Outdated
@@ -507,6 +507,28 @@ describe('initialization options', () => { | |||
expect(client.getStringAssignment(flagKey, 'subject', {}, 'default-value')).toBe('control'); | |||
}); | |||
|
|||
it('skips initial poll', async () => { |
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.
🙌
assignmentLogger: mockLogger, | ||
skipInitialPoll: true, | ||
}); | ||
expect(callCount).toBe(0); |
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.
👍
src/index.ts
Outdated
/** | ||
* Skip the first poll for new configurations during initialization. (default: false) | ||
*/ | ||
skipInitialPoll?: boolean; |
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.
🔥
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.
Just a thought: "poll" implies a request made repeatedly at intervals. skipInitialRequest
?
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.
Good idea - I was being too pedantic with the mechanics of the internal poller. For customers they are not going to know that and the naming you suggested is more clear 👍 👍
src/index.ts
Outdated
@@ -160,6 +165,7 @@ export async function init(config: IClientConfig): Promise<IEppoClient> { | |||
pollAfterSuccessfulInitialization: config.pollAfterSuccessfulInitialization ?? false, | |||
pollAfterFailedInitialization: config.pollAfterFailedInitialization ?? false, | |||
throwOnFailedInitialization: true, // always use true here as underlying instance fetch is surrounded by try/catch | |||
skipInitialPoll: config.skipInitialPoll ?? false, |
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.
👍
labels: mergeable
Fixes: #issue
Motivation and Context
Customers can elect to put the client SDK into "offline" mode - one of the configurations they need to enable is preventing the SDK from performing a CDN fetch during the
init
method.Subsequent calls to the CDN will be blocked by setting the
isExpired
function in the persistence store implementations.Description
skipInitialPoll
How has this been tested?