feat(server): YAML config file support #7894
Conversation
| @@ -341,19 +342,19 @@ export class SystemConfigCore { | |||
| if (force || !this.configCache) { | |||
| try { | |||
| const file = await this.repository.readFile(filepath); | |||
| const json = JSON.parse(file.toString()); | |||
| const config = loadYaml(file.toString()) as any; | |||
There was a problem hiding this comment.
js-yaml has this option:
json(default: false) - compatibility with JSON.parse behaviour. If true, then duplicate keys in a mapping will override values rather than throwing an error.
I don't think we need it, right?
There was a problem hiding this comment.
Yeah, that probably doesn't matter. I don't think it matters how exactly the parser works, just that it will still have to pass validation with class-transformer/class-validator either way. This seems like a more edge-case for parsing that we don't care about. Although, it would be great to add an e2e test for this too. I can probably do that if you don't want to both with it.
There was a problem hiding this comment.
Oh I didn't realize you could unit test it so easily, but that works too.
| if (!_.isEmpty(json)) { | ||
| this.logger.warn(`Unknown keys found: ${JSON.stringify(json, null, 2)}`); | ||
| if (!_.isEmpty(config)) { | ||
| this.logger.warn(`Unknown keys found: ${JSON.stringify(config, null, 2)}`); |
There was a problem hiding this comment.
I left this as just JSON output rather than trying to format based on what the input was, to keep the code simple.
There was a problem hiding this comment.
I think that's fine, it is just a warning anyways and the user should be able to map it back themselves.
Deploying with
|
| Latest commit: |
26984e5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b7c27d26.immich.pages.dev |
| Branch Preview URL: | https://feat-yaml-config.immich.pages.dev |
No description provided.