Check config before starting MM by KristjanESPERANTO · Pull Request #3450 · MagicMirrorOrg/MagicMirror

KristjanESPERANTO · 2024-05-14T00:06:25Z

I think it might be a good idea to check the config at every start.

khassel · 2024-05-14T20:03:56Z

We implemented the possibility to use a config.js.template instead of a config.js (where the config.js is created while startup).

At the moment the check fails if there is no config.js:

[2024-05-14 21:51:17.549] [ERROR] File not found: /opt/magic_mirror/config/config.js

So if we want this check the code must be changed that no error is thrown if no config.js exists but a config.js.template.
If we want to check config.js.template in such a case I don't now if the check can handle Variables ${XY} which can be present in config.js.template. There is also a env variable MM_CONFIG_FILE which is used to set another config file location. I don't know if the check respects this variable (it should).

Other point: Any idea why the test fails?

bugsounet · 2024-05-19T11:43:06Z

Why not hard coding this check in main core of MM² before loading config ?

in this case, it's +/- related to #3445

sdetweil · 2024-05-19T12:54:26Z

in general I am in favor of not starting if there is a fatal config error .

not like now where we start with a built in dummy config, unlike whatever the user expected.

khassel · 2024-05-19T13:16:37Z

I agree with the idea, I just wanted to point out that the current approach doesn't fit all use cases...

Why not hard coding this check in main core of MM² before loading config ?

I think loadConfig in app.js must be executed before the check because in this function the config.js is created when using config.js.template.

KristjanESPERANTO · 2024-05-24T08:18:12Z

I'd like to take a closer look, but it looks like I will not find the time soon.

KristjanESPERANTO · 2024-09-18T01:36:15Z

I think loadConfig in app.js must be executed before the check because in this function the config.js is created when using config.js.template.

The config check is now called from inside the loadConfig after handling the config file. So I think it should now cover all use cases:

normal config.js
MM_CONFIG_FILE and
config.js.template

in general I am in favor of not starting if there is a fatal config error

Now if the config check fails, MM will not start:

kristjan@debian:~/MagicMirror$ npm run start

> [email protected] start
> DISPLAY="${DISPLAY:=:0}" ./node_modules/.bin/electron js/electron.js

[2024-09-18 03:31:25.980] [LOG]   Starting MagicMirror: v2.29.0-develop 
[2024-09-18 03:31:25.999] [LOG]   Loading config ... 
[2024-09-18 03:31:26.001] [LOG]   config template file not exists, no envsubst 
[2024-09-18 03:31:26.319] [INFO]  Checking file...  /home/kristjan/MagicMirror/config/config.js 
[2024-09-18 03:31:26.343] [ERROR] Your configuration file contains syntax errors :( 
[2024-09-18 03:31:26.343] [ERROR] Line 193 column 3: Parsing error: Unexpected token { 
kristjan@debian:~/MagicMirror$

khassel · 2024-09-18T17:17:19Z

@KristjanESPERANTO can you please solve the merge conflicts, thanks!

…ve (#3565) after adding check:config to the MM startup process, #3450, we accidentally discover module positions more than once, and write the file each time.. add a check to see if we have done this work already

@bugsounet

## [2.29.0] - 2024-10-01 Thanks to: @bugsounet, @dkallen78, @jargordon, @khassel, @KristjanESPERANTO, @MarcLandis, @rejas, @ryan-d-williams, @sdetweil, @skpanagiotis. > ⚠️ This release needs nodejs version `v20` or `v22`, minimum version is `v20.9.0` ### Added - [compliments] Added support for cron type date/time format entries mm hh DD MM dow (minutes/hours/days/months and day of week) see https://crontab.cronhub.io for construction (#3481) - [core] Check config at every start of MagicMirror² (#3450) - [core] Add spelling check (cspell): `npm run test:spelling` and handle spelling issues (#3544) - [core] removed `config.paths.vendor` (could not work because `vendor` is hardcoded in `index.html`), renamed `config.paths.modules` to `config.foreignModulesDir`, added variable `MM_CUSTOMCSS_FILE` which - if set - overrides `config.customCss`, added variable `MM_MODULES_DIR` which - if set - overrides `config.foreignModulesDir`, added test for `MM_MODULES_DIR` (#3530) - [core] elements are now removed from `index.html` when loading script or stylesheet files fails - [core] Added `MODULE_DOM_UPDATED` notification each time the DOM is re-rendered via `updateDom` (#3534) - [tests] added minimal needed node version to tests (currently v20.9.0) to avoid releases with wrong node version info - [tests] Added `node-libgpiod` library to electron-rebuild tests (#3563) ### Removed - [core] removed installer only files (#3492) - [core] removed raspberry object from systeminformation (#3505) - [linter] removed `eslint-plugin-import`, because it doesn't support ESLint v9. We will reenter it later when it does. - [tests] removed `onoff` library from electron-rebuild tests (#3563) ### Updated - [weather] Updated `apiVersion` default from 2.5 to 3.0 (#3424) - [core] Updated dependencies including stylistic-eslint - [core] nail down `node-ical` version to `0.18.0` with exception `allow-ghsas: GHSA-8hc4-vh64-cxmj` in `dep-review.yaml` (which should removed after next `node-ical` update) - [core] Updated SocketIO catch all to new API - [core] Allow custom modules positions by scanning index.html for the defined regions, instead of hard coded (PR #3518 fixes issue #3504) - [core] Detail optimizations in `config_check.js` - [core] Updated minimal needed node version in `package.json` (currently v20.9.0) (#3559) and except for v21 (no security updates) (#3561) - [linter] Switch to ESLint v9 and flat config and replace `eslint-plugin-unicorn` by `@eslint/js` - [core] fix discovering module positions twice after #3450 ### Fixed - Fixed `checks` badge in README.md - [weather] Fixed issue with the UK Met Office provider following a change in their API paths and header info. - [core] add check for node_helper loading for multiple instances of same module (#3502) - [weather] Fixed issue for respecting unit config on broadcasted notifications - [tests] Fixes calendar test by moving it from e2e to electron with fixed date (#3532) - [calendar] fixed sliceMultiDayEvents getting wrong count and displaying incorrect entries, Europe/Berlin (#3542) - [tests] ignore `js/positions.js` when linting (this file is created at runtime) - [calendar] fixed sliceMultiDayEvents showing previous day without config enabled --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Michael Teeuw <[email protected]> Co-authored-by: Kristjan ESPERANTO <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ross Younger <[email protected]> Co-authored-by: Veeck <[email protected]> Co-authored-by: Bugsounet - Cédric <[email protected]> Co-authored-by: jkriegshauser <[email protected]> Co-authored-by: illimarkangur <[email protected]> Co-authored-by: sam detweiler <[email protected]> Co-authored-by: vppencilsharpener <[email protected]> Co-authored-by: veeck <[email protected]> Co-authored-by: Paranoid93 <[email protected]> Co-authored-by: Brian O'Connor <[email protected]> Co-authored-by: WallysWellies <[email protected]> Co-authored-by: Jason Stieber <[email protected]> Co-authored-by: jargordon <[email protected]> Co-authored-by: Daniel <[email protected]> Co-authored-by: Ryan Williams <[email protected]> Co-authored-by: Panagiotis Skias <[email protected]> Co-authored-by: Marc Landis <[email protected]>

Check config before starting MM

2989831

KristjanESPERANTO added 3 commits September 18, 2024 03:00

Check after creating config from template

a5ce58a

Merge branch 'develop' into checkconfig

34dcc34

Undo some formating to keep the PR clean

e48b992

Merge branch 'develop' into checkconfig

50353a2

khassel approved these changes Sep 18, 2024

View reviewed changes

khassel merged commit 866419e into MagicMirrorOrg:develop Sep 18, 2024

KristjanESPERANTO deleted the checkconfig branch September 18, 2024 17:38

bugsounet mentioned this pull request Sep 28, 2024

add support for custom regions, by detecting what is used in index.html #3518

Merged

sdetweil mentioned this pull request Sep 28, 2024

fix double load of positions now that check:config at startup is active #3565

Merged

khassel mentioned this pull request Sep 30, 2024

Release 2.29.0 #3568

Merged

timmymac-tech mentioned this pull request Oct 6, 2024

Release 2.29.0 (#3568) timmymac-tech/MagicMirror#2

Merged

Protonosgit mentioned this pull request Oct 6, 2024

Release 2.29.0 (#3568) Protonosgit/MagicMedia#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Check config before starting MM#3450

Check config before starting MM#3450
khassel merged 5 commits intoMagicMirrorOrg:developfrom
KristjanESPERANTO:checkconfig

KristjanESPERANTO commented May 14, 2024

Uh oh!

khassel commented May 14, 2024

Uh oh!

bugsounet commented May 19, 2024 •

edited

Loading

Uh oh!

sdetweil commented May 19, 2024

Uh oh!

khassel commented May 19, 2024

Uh oh!

KristjanESPERANTO commented May 24, 2024

Uh oh!

KristjanESPERANTO commented Sep 18, 2024

Uh oh!

khassel commented Sep 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

KristjanESPERANTO commented May 14, 2024

Uh oh!

khassel commented May 14, 2024

Uh oh!

bugsounet commented May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdetweil commented May 19, 2024

Uh oh!

khassel commented May 19, 2024

Uh oh!

KristjanESPERANTO commented May 24, 2024

Uh oh!

KristjanESPERANTO commented Sep 18, 2024

Uh oh!

khassel commented Sep 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bugsounet commented May 19, 2024 •

edited

Loading