feat(splitio-api-binding): add function to poll for split changes #5
Conversation
lib/splitio-api-binding.js
Outdated
| const allChanges = [results] | ||
| // The till value represents the timestamp of the last change included in the response. | ||
| let till = results.till | ||
| while (since !== till) { |
There was a problem hiding this comment.
Are these values guaranteed to converge? I am worried that there would be a case where this runs infinitely.
There was a problem hiding this comment.
i think they will -- it would only run a long time if there's a huge number splits
from the doc from split:
Ideally you would keep fetching changes until this condition is fulfilled, instead of just assuming one request will get you everything.
After this condition is true, you periodically poll for updates.
There will only be updates to the data if this condition is NOT true on a given response.
There was a problem hiding this comment.
The potentially infinite loop makes me nervous. E.g., if split.io API has a bug, that bubbles up as a infinite loop in our product code. Can we add a limit on the loop?
| allChanges.forEach(change => { | ||
| splitChanges.push(change.splits) | ||
| }) | ||
| return { splitChanges, since } |
There was a problem hiding this comment.
I wonder why this function returns since, will we use it somewhere else?
There was a problem hiding this comment.
yep, the FE data loader will use it so that the SDK knows what timestamp to use for polling for updates
There was a problem hiding this comment.
edit: talked to silas more about this -- we'll eventually keep track of this on the backend but are just going to poll with -1 for now
|
closing for now, need to make a couple changes |
lib/splitio-api-binding.js
Outdated
| const allChanges = [results] | ||
| // The till value represents the timestamp of the last change included in the response. | ||
| let till = results.till | ||
| while (since !== till) { |
There was a problem hiding this comment.
does this work?
const since = SINCE_VALUE_FOR_FIRST_REQUEST
const allChanges = []
while (true) {
const results = await this.httpGet({ path, since })
if (since === results.till) {
break
}
allChanges.push(results)
since = results.till
}There was a problem hiding this comment.
ah yes, so much nice
lib/splitio-api-binding.js
Outdated
| const allChanges = [results] | ||
| // The till value represents the timestamp of the last change included in the response. | ||
| let till = results.till | ||
| while (since !== till) { |
There was a problem hiding this comment.
The potentially infinite loop makes me nervous. E.g., if split.io API has a bug, that bubbles up as a infinite loop in our product code. Can we add a limit on the loop?
This reverts commit cea35a7.
No description provided.