Events Batch Processing flow

[emilianofant-split]

Javascript Slim Synchronizer

What did you accomplish?

In this PR I'm adding:

• The config to determine the batch size of elements to POP from Storage (Events)
• Wrap the pop function into a function that can call itself recursively until there are no more elements in the Storage.
• Added CLI config and manager capability to create an Event Synchroniser with a custom batch size value.

How do we test the changes introduced in this PR?

1 - Checkout this branch:
git checkout efant_eventsImpressions_retries_batchProcess
2 - Run unit tests
npm run test:unit

If you want to test it manually:
This file is a Custom Storage to connect with Redis, and can be used for manual tests.
For example:

npm run build && clear && ./dist/cli.js -k <A_VALID_APIKEY> -s ../path/to/inRedisStorage.cjs -r https://sdk.split-stage.io/api -e https://events.split-stage.io/api -p storagePrefix -n 100

Note the path to the Storage file

Related JIRA tickets and PRs

[Synchroniser] - Set retries strategy for Impressions/Events sync

Extra Notes

Next steps:

• Add option to configure how many batches should be processed, or until 0 elements are in storage (by default)

[EmilianoSanchez]

Just a comment about parsing and validating the new eventsPerPost config param. If you prefer, you can update it in another PR, but don't forget to add a TODO comment or to your checklist.

[EmilianoSanchez]

is this param used?. If events submitter ATTEMPTS_NUMBER is fixed at the moment, maybe we should remove it or at least put some comment to the eventsRequestRetriesAmount config param.

[emilianofant-split]

I've forgotten to implement it into the submitters. I've just committed the changes.

[emilianofant-split]

7cccfa7

[EmilianoSanchez]

Not sure how yargs works in detail, but if EVENTS_PER_POST is a string, we should parse it into a number (eventsPerPost is a number) and validate it. The same for other places where we must parse a string into a number.

Just as an example to explain it better:

value = parseInt(env.EVENTS_PER_POST, 10);
if (isNaNNumber(value) || value <= 0) {
   log.warn('EVENTS_PER_POST must be a positive integer number. Defaulting to ${DEFAULT_EVENTS_PER_POST');
   synchronizerConfigs.eventsPerPost = DEFAULT_EVENTS_PER_POST
} else {
   synchronizerConfigs.eventsPerPost = value;
}

[emilianofant-split]

Good catch, makes sense to preventively parse it to Number in case the value is get from .env or json file.

[EmilianoSanchez]

Just a comment, to consider when reviewing all together the public API, maybe this param should be renamed to eventBatchSizePerPost or something like that.

[emilianofant-split]

I agree, but right now I'm applying the same API naming from https://help.split.io/hc/en-us/articles/360019686092-Split-synchronizer-proxy#common-configuration-synchronizer-and-proxy-mode
We should discuss this with the team in order to agree in a common interface for all the synchronizers