Add icuTokenizer as allowed field into configValidation.js

[SiarheiFedartsou]

👋 I did some awesome work for the Pelias project and would love for everyone to have a look at it and provide feedback.

---

Here's the reason for this change 🚀

We added this field into config recently in pelias/schema and it looks like it has to be in sync with what have here, otherwise validation of config doesn't pass:

Error: "schema.icuTokenizer" is not allowed
    at Object.validate (/code/pelias/csv-importer/node_modules/pelias-dbclient/src/configValidation.js:27:13)
    at Object.<anonymous> (/code/pelias/csv-importer/node_modules/pelias-dbclient/index.js:2:37)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/code/pelias/csv-importer/lib/importPipeline.js:4:24)
    at Module._compile (internal/modules/cjs/loader.js:999:30)

---

Here's what actually got changed 👏

Just added definition of icuTokenizer field in this repo.

---

Here's how others can test the changes 👀

I extended config validation tests.

[orangejulius]

Honestly, what we should probably do instead is create a config.featureFlags namespace that allows any property with a boolean. Not sure if our schema validation supports that, but I don't want to have to make a new PR across several Pelias repos for any future flags. What do you think @missinglink?

[missinglink]

Hmm.. when we merged the schema PR I didn't consider that the new config variable would need to be added in several places across the codebase.

I agree this is not a sustainable approach to BETA features under a feature flag as it creates a lot of noise in the repos when adding and removing these flags.

@orangejulius suggested a better approach, namely adding a .featureFlags object to the schema which was of type Object<string, boolean>, the validation would then accept any key as long as the value was boolean.

This approach would allow us to make this change once and then it will 'just-work' without any modification for future BETA features, as well as allowing us to remove those which get promoted to the main distribution without modifying the code.

@orangejulius I guess this would be a top-level property in pelias/schema? as it could potentially apply to several different parts of Pelias.

The ICU tokenizer BETA flag was placed in config.schema.icuTokenizer, so this would now become config.featureFlags.icuTokenizer=true?

[SiarheiFedartsou]

Hey, I agree with your concerns. I will create separate PR in pelias/schema then...

[orangejulius]

Hmm, well also, looking at the config validation code, the Joi schma IMO should not really be validating what's in the Pelias schema config section. Maybe we can make anything in the schema section of pelias config optional and unvalidated? I don't remember why we did it this way, but it seems like validating schema settings should not be the responsibility of dbclient.

dbclient/src/configValidation.js

Lines 9 to 20 in a8c22d1

	const schema = Joi.object().keys({
	dbclient: Joi.object().required().keys({
	statFrequency: Joi.number().integer().min(0).required(),
	batchSize: Joi.number().integer().min(0).required()
	}),
	esclient: Joi.object().required().keys({
	requestTimeout: Joi.number().integer().min(0)
	}).unknown(true),
	schema: Joi.object().keys({
	indexName: Joi.string().required()
	})
	}).unknown(true);

[SiarheiFedartsou]

pelias/schema#505

WDYT? :)