fix(schema): correct the last 8 restart flags — v0.48.0 - #37
Merged
Conversation
Closes the audit begun in 0.46.0. All 72 keys have now been traced to a
consumer: 49 "restart": true, 23 "restart": false, none unverified.
0.47.0 deliberately left these eight alone because a literal-key search found
no SettingsRepository::getEffective() consumer -- and after
port-forward.port_forwarding.upnp_enabled turned out INERT rather than
mis-flagged, guessing was not acceptable. Tracing them showed why the search
missed them: they are not read through SettingsRepository at all.
* webhooks.enabled, stats.enabled -- read via EffectiveConfig::file(). That
LOOKS live because the call happens at use time, but EffectiveConfig's
$overrides is a STATIC array populated once by bootstrap(), which runs in
onWorkerStart, and file() is memoised per bootstrap generation. A saved
override is invisible to a running worker whenever file() is called.
* transcoding.tone_mapping_mode / .prefer_hdr_output -- merged by
HwAccelConfig::get() and handed to FfmpegRunner::setConfig(): the IDENTICAL
path as transcoding.preferred_accelerator and the hwaccel.* keys, which
already carried "restart": true. Sibling keys, same config file, same
consumer, contradictory flags.
* newsletter.enabled / .send_hour -- read from the boot $config array when
the newsletter timer is registered, once per worker start.
* relay.reconnect_delay / .ping_interval -- captured into the RelayConfig DI
factory, whose result PHP-DI caches per container.
"restart": true here means the admin Restart control, which sends a graceful
reload (SIGUSR2): workers cycle, re-run onWorkerStart, re-bootstrap the overlay
and rebuild their DI containers. A key needing that cycle is "restart": true
even when a full systemctl restart is not.
The 23 keys still at "restart": false all resolve through getEffective()/
getOverride() at USE time -- an uncached SELECT per call, genuinely live.
No key added or removed; property count unchanged at 72.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
🟢 Coverage ∅ diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation (-1.00%) Diff coverage ✅ ∅ diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (d6d07ee) 1016 829 81.59% Head commit (45a87c5) 1016 (+0) 829 (+0) 81.59% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#37) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #37 +/- ##
=========================================
Coverage 81.29% 81.29%
Complexity 414 414
=========================================
Files 50 50
Lines 1160 1160
=========================================
Hits 943 943
Misses 217 217
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the audit begun in #35 and continued in #36. All 72 keys are now traced to a consumer: 49
restart: true, 23restart: false, none unverified.Why these eight were held back
#36 deliberately left them alone: a literal-key search found no
SettingsRepository::getEffective()consumer for any of them, and afterport-forward.port_forwarding.upnp_enabledturned out to be inert rather than mis-flagged, guessing a flag was not acceptable. Tracing them explains the empty search — they are not read throughSettingsRepositoryat all.webhooks.enabled,stats.enabledEffectiveConfig::file()EffectiveConfig::$overridesis a static array populated once bybootstrap(), which runs inonWorkerStart, andfile()is memoised per bootstrap generation. A saved override is invisible to a running worker regardless of whenfile()is called.transcoding.tone_mapping_mode,transcoding.prefer_hdr_outputHwAccelConfig::get()→FfmpegRunner::setConfig()transcoding.preferred_acceleratorand thehwaccel.*keys, which already carriedrestart: true. Sibling keys, same config file, same consumer, contradictory flags.newsletter.enabled,newsletter.send_hour$config['newsletter']relay.reconnect_delay,relay.ping_intervalRelayConfig::class => factory(…)over$appConfig['relay']None of them is inert; every one has a real consumer.
stats.enabled's own docblock already said the override "applies on reload" — which is precisely what this flag advertises.What
restart: truemeans herePOST /api/v1/admin/restartsends a graceful reload (SIGUSR2): workers cycle, re-runonWorkerStart, re-bootstrap the overlay, and rebuild their DI containers. A key that needs that cycle isrestart: trueeven when a fullsystemctl restartis not required. That is the distinction separating these eight from the 23 below.The 23 that stay
restart: falseAll resolve through
SettingsRepository::getEffective()/getOverride()at use time — an uncachedSELECTper call, genuinely live:auth.signup_mode·auth.password.min_length·auth.access_ttl·auth.refresh_ttl·auth.max_profiles·access.default_concurrent_streams·transcoding.preset·transcoding.crf_h264·transcoding.audio_bitrate·artwork.download_enabled·scanner.ignore_patterns·subtitles.default_language·subtitles.provider_priority·trickplay.enabled·trakt.client_id·trakt.client_secret·trakt.redirect_uri·metadata.overwrite_existing·casting.chromecast.enabled·casting.roku.enabled·casting.airplay.enabled·dlna.allowed_cidrs·dlna.restrict_to_lanThat list is exactly what remains at
restart: falseafter this change — a clean cross-check that the audit closes.Scope
No key added or removed; property count unchanged at 72. Diff is 16 lines changed across 8 keys (
restart+helpTexteach), verified withgit diff | grep -vc '"restart"|"helpText"'→ 0 unintended lines.⚠ Worth recording: the first pass of the edit script flipped only 5 of 8 — three keys carry
"restart": falseas the object's last property, so the line has no trailing comma and did not match the pattern. Caught by asserting the count rather than trusting the run.Verification
./vendor/bin/phpunitcomposer stancomposer cs🤖 Generated with Claude Code