feat(web, server): Ability to use config file instead of admin UI by danieldietzler · Pull Request #3836 · immich-app/immich

danieldietzler · 2023-08-22T11:41:30Z

This PR closes #3774, providing the ability to use a JSON config file instead of the admin UI.

Currently, the admin UI and the config file are mutually exclusive while the config file always takes precedence. So, if a config file is specified, changes in the admin UI are not possible anymore.
To get people started with this quickly, there is a new copy to clipboard button that allows to copy the entire config as a formatted JSON string.

If also added documentation for the new env variable, but I wasn't sure where to put the documentation for the config file.

…is set, copy existing config to clipboard

vercel · 2023-08-22T11:41:34Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
immich	⬜️ Ignored (Inspect)	Visit Preview		Aug 25, 2023 5:01pm

mertalev

Thanks for picking this up!

jrasm91 · 2023-08-22T14:37:10Z

This looks really good. Looking at it now, I'm wondering if maybe it would be better to move the read config file method to the config repository instead of the storage repository, since that would reduce a lot of unnecessary changes in this PR.

danieldietzler · 2023-08-22T14:42:00Z

Yeah, that was quite an adventure... :D
~~I asked about this beforehand (in the issue) and this was suggested. This is my first suggestion, so I am really new to the code base and don't know where methods make the most sense.~~ (Sorry, I did not notice it was you @jrasm91... @mertalev and you have quite a similar profile pic and I did not read the names)

So, if you'd say that it would make more sense in the config repository, I sure can change it! This would make this PR a lot slimmer and simpler ;)
Alternatively, we can also wait for others to chime in, maybe there are some people who have opinions on this.

danieldietzler · 2023-08-22T14:42:46Z

At least I got to know the code base a bit by this ^^

jrasm91 · 2023-08-22T14:49:01Z

Yeah, I don't think there are strong preferences either way. I'm probably the primary developer in this area of the codebase and I'm good either way. I think it would be simpler myself and thought of it after posting my comment yesterday. Usually it is nice to keep related functions together, so reading a file naturally could belong with all the other file operations in the storage repository, but in this case that makes that change quite unwieldy and I don't see much benefit requiring it to be there. I usually prefer simplicity and moving it to the config repository would make it considerably simpler.

danieldietzler · 2023-08-22T14:55:32Z

On the other hand, now that I've refactored the method to read files in general, if there should be some other use case at some time where reading files would be needed again, the logic would then already be in the right place. So, it would make the life of future devs easier :)

jrasm91 · 2023-08-22T15:25:12Z

True although, it is trivial to add that again if needed 😛

danieldietzler · 2023-08-22T15:31:24Z

Sure, but this would result in duplicate code unless you streamline the config file process then, at which point you would have a bit more to do.

bo0tzz

Thanks for picking this up!

I wasn't sure where to put the documentation for the config file.

This would probably suit a new page under docs/docs/install, after the env vars page.

bo0tzz · 2023-08-22T19:27:54Z

As for the file reading discussion: Reading a file from disk is a relatively trivial event, so I don't think we need to anticipate much future need there. It's probably best to just take the simplest approach/with the least changes needed.

mertalev

I think it's good once isConfigFile and validation are sorted out.

alextran1502 · 2023-08-23T06:06:28Z

Hello, can you please include a screenshot of the UI elements?

danieldietzler · 2023-08-24T13:38:53Z

Why did the intl version change just from running make api? I don't think I should change the version in the pubspec.yaml, right?

mertalev · 2023-08-25T03:24:57Z

Not sure, but your openapi-generator version might be different than what's on main. I'd run npm ci in server and try again.

jrasm91 · 2023-08-25T03:27:14Z

I think this is actually pinned in a patch file that probably isn't getting applied properly.

danieldietzler · 2023-08-25T10:52:35Z

Not sure, but your openapi-generator version might be different than what's on main. I'd run npm ci in server and try again.

That unfortunately did not do the trick.

danieldietzler · 2023-08-25T10:55:39Z

I think this is actually pinned in a patch file that probably isn't getting applied properly.

Sorry, but I can't seem to find any patch file that would be related to this. Also, they are generated and should not be touched either, right?
What do you propose I try, or are you debugging yourself already and trying to fix the issue?

jrasm91 · 2023-08-25T16:17:15Z

Added a few things:

Renamed to IMMICH_CONFIG_FILE
Show a notification after clicking copy to clipboard
Add a button to export to json
Validate the file (after merging it with the default configuration). This also throws errors for unknown keys
Always show copy/export buttons, even when config is disabled.

Outdated

danieldietzler · 2023-08-25T17:05:09Z

Thanks a lot! :)

bo0tzz · 2023-08-25T18:49:56Z

Thank you for the PR!

) * implement method to read config file * getConfig returns config file if present * return isConfigFile for http requests * disable elements if config file is used, show message if config file is set, copy existing config to clipboard * fix allowing partial configuration files * add new env variable to docs * fix tests * minor refactoring, address review * adapt config type in frontend * remove unnecessary imports * move config file reading to system-config repo * add documentation * fix code formatting in system settings page * add validator for config file * fix formatting in docs * update generated files * throw error when trying to update config. e.g. via cli or api * switch to feature flags for isConfigFile * refactoring * refactor: config file * chore: open api * feat: always show copy/export buttons * fix: default flags * refactor: copy to clipboard --------- Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

danieldietzler added 7 commits August 22, 2023 10:22

implement method to read config file

8c18f38

getConfig returns config file if present

6229da4

return isConfigFile for http requests

6e2e534

disable elements if config file is used, show message if config file …

483e020

…is set, copy existing config to clipboard

fix allowing partial configuration files

796263f

add new env variable to docs

efbcb2b

fix tests

fa922b7

danieldietzler changed the title ~~feat(web, server): Use a config file instead of the admin UI~~ feat(web, server): Ability to use config file instead of admin UI Aug 22, 2023

mertalev requested changes Aug 22, 2023

View reviewed changes

minor refactoring, address review

1524efe

danieldietzler added 2 commits August 22, 2023 16:58

adapt config type in frontend

e6e4a12

remove unnecessary imports

b4e346b

danieldietzler force-pushed the main branch from 486b87a to b4e346b Compare August 22, 2023 15:02

danieldietzler requested a review from mertalev August 22, 2023 16:45

bo0tzz requested changes Aug 22, 2023

View reviewed changes

Comment thread server/src/domain/system-config/system-config.core.ts Outdated

Comment thread server/src/domain/system-config/dto/system-config.dto.ts Outdated

danieldietzler added 2 commits August 22, 2023 22:17

move config file reading to system-config repo

47534d9

add documentation

ed6cb8d

vercel Bot temporarily deployed to Preview August 22, 2023 20:42 Inactive

fix code formatting in system settings page

9e28771

mertalev previously requested changes Aug 23, 2023

View reviewed changes

Comment thread server/src/domain/system-config/dto/system-config.dto.ts Outdated

Comment thread server/src/domain/system-config/system-config.core.ts Outdated

danieldietzler and others added 2 commits August 23, 2023 12:49

Merge branch 'immich-app:main' into main

36d1208

fix formatting in docs

a2ff5d6

vercel Bot temporarily deployed to Preview August 23, 2023 19:22 Inactive

update generated files

1d13816

danieldietzler and others added 2 commits August 24, 2023 20:27

throw error when trying to update config. e.g. via cli or api

e0d2aa2

Merge branch 'immich-app:main' into main

ac4a9e4

danieldietzler and others added 5 commits August 25, 2023 13:04

Merge branch 'immich-app:main' into main

1371530

Merge branch 'immich-app:main' into main

20ded69

switch to feature flags for isConfigFile

2493499

Merge branch 'immich-app:main' into main

c194c74

refactoring

00b38e6

bo0tzz approved these changes Aug 25, 2023

View reviewed changes

Comment thread server/src/domain/server-info/server-info.dto.ts Outdated

jrasm91 added 4 commits August 25, 2023 11:56

refactor: config file

8abcf06

chore: open api

d62ee6f

feat: always show copy/export buttons

21ea7cc

fix: default flags

2c45706

jrasm91 approved these changes Aug 25, 2023

View reviewed changes

jrasm91 removed the request for review from mertalev August 25, 2023 16:19

refactor: copy to clipboard

c64a5b1

jrasm91 merged commit 59bb727 into immich-app:main Aug 25, 2023

Conversation

danieldietzler commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mertalev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrasm91 commented Aug 22, 2023

Uh oh!

danieldietzler commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danieldietzler commented Aug 22, 2023

Uh oh!

jrasm91 commented Aug 22, 2023

Uh oh!

danieldietzler commented Aug 22, 2023

Uh oh!

jrasm91 commented Aug 22, 2023

Uh oh!

danieldietzler commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bo0tzz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bo0tzz commented Aug 22, 2023

Uh oh!

mertalev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alextran1502 commented Aug 23, 2023

Uh oh!

danieldietzler commented Aug 24, 2023

Uh oh!

mertalev commented Aug 25, 2023

Uh oh!

jrasm91 commented Aug 25, 2023

Uh oh!

danieldietzler commented Aug 25, 2023

Uh oh!

danieldietzler commented Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jrasm91 commented Aug 25, 2023

Uh oh!

danieldietzler commented Aug 25, 2023

Uh oh!

bo0tzz commented Aug 25, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danieldietzler commented Aug 22, 2023 •

edited

Loading

vercel Bot commented Aug 22, 2023 •

edited

Loading

danieldietzler commented Aug 22, 2023 •

edited

Loading

danieldietzler commented Aug 22, 2023 •

edited

Loading

danieldietzler commented Aug 25, 2023 •

edited

Loading