luci-app-adguardhome: add new app#8413
Conversation
GeorgeSapkin
force-pushed
the
add-luci-app-adguardhome
branch
9 times, most recently
from
e58b138 to
ba94f94
Compare
|
Another yaml config monster :) Those go variables are globals, yes? |
Not sure I understand what you mean by globals here. These are per-process. |
GeorgeSapkin
force-pushed
the
add-luci-app-adguardhome
branch
from
ba94f94 to
ef2d48b
Compare
GeorgeSapkin
force-pushed
the
add-luci-app-adguardhome
branch
2 times, most recently
from
91cf2e1 to
7acacb3
Compare
|
Per-process, implying they control the behaviour of all Go binaries on the system? |
They can be set for each Go process individually. They could be set on a system-level, but that doesn't make sense. Here's how it's used for AGH: [ "$gc" -le 0 ] || procd_append_param env GOGC="$gc"
[ "$maxprocs" -le 0 ] || procd_append_param env GOMAXPROCS="$maxprocs"
[ "$memlimit" -le 0 ] || procd_append_param env GOMEMLIMIT="$memlimit" |
|
Ah, OK. That looks like it's set only for the agh sub-process. Is this ready, then? |
|
If nothing jumps out as wrong. Particularly around polling and updating a UI element. I couldn't figure out something that looked idiomatic to update the Service Status field. |
GeorgeSapkin
force-pushed
the
add-luci-app-adguardhome
branch
3 times, most recently
from
82c035d to
3fdff94
Compare
| advSettingsOpt.load = () => sessionStorage.getItem(STORAGE_KEY) || '0'; | ||
| advSettingsOpt.remove = () => {}; | ||
| advSettingsOpt.write = (_, value) => sessionStorage.setItem(STORAGE_KEY, value); |
There was a problem hiding this comment.
This is a bit odd. Why store a key for agh in a different uci config file?
There was a problem hiding this comment.
This is a session storage entry that marks that advanced settings tab is visible. Nothing is stored in UCI.
There was a problem hiding this comment.
So user logs out and in and the settings are hidden anew?
There was a problem hiding this comment.
The retain = 1 should help, otherwise the dependent settings get erased.
There was a problem hiding this comment.
I'm not familiar with all the knobs yet. Can you make a small snippet of what needs to be modified in this case?
There was a problem hiding this comment.
Do you want to store something permanently or only while you're logged in?
There was a problem hiding this comment.
Something for the duration of the session, hence why I used the session storage. But while logged in also works if that's more idiomatic.
There was a problem hiding this comment.
Then what you have is fine. It's not idiomatic with conventions in this repo so others might find this and think it's a bug.
applications/luci-app-adguardhome/htdocs/luci-static/resources/view/adguardhome/config.js
Show resolved
Hide resolved
Add LuCI UI for AdGuard Home configuration. If AdGuard Home service is running, restart it automatically when configuration is applied. Signed-off-by: George Sapkin <george@sapk.in>
systemcrash
force-pushed
the
add-luci-app-adguardhome
branch
from
3fdff94 to
b43040a
Compare
|
Let's see how she fares. master only? |
Yeah, it depends on config changes on the service side that are not in stable yet. |
2 participants
Add LuCI UI for AdGuard Home configuration.
If AdGuard Home service is running, restart it automatically when configuration is applied.
Depends on:
Run Testing Details