[Argus] Fix config parsing bug#4788
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
Would merging #4759 without this fix be problematic? |
The error fixed in this PR occurred if we didn't provide the required property of an optional object in config.yml. In #4759, a new property |
| public constructor(message: string, errors: Ajv['errors']) { | ||
| const errorMessages: string[] = [] | ||
| errors?.forEach((e) => errorMessages.push(`${e.dataPath}: ${e.message} (${JSON.stringify(e.params)})`)) | ||
| errors?.forEach((e) => errorMessages.push(`${e.instancePath}: ${e.message} (${JSON.stringify(e.params)})`)) |
There was a problem hiding this comment.
I see this change from dataPath to instancePath is due to newer version of ajv package. https://ajv.js.org/api.html#validation-errors
But storage-node also uses ajv for schema validation so we may need to do a similar fix there
There was a problem hiding this comment.
Both storage-node & distributor-node have in-package import of ajv dependency from storage-node/node_modules & distributor-node/node_modules/ as it's added separately in storage-node/package.json & distributor-node/package.json.
- However, I have updated
ajvdependency to^8..0.0in/storage-node&/cli - Removed
ajvdependency from/types, as it's unused
addresses #4787
#4787in 695c66a. The fix allows omitting optional config keys fromconfig.ymlfile when specifying their values as env vars.