Feat: Allow selected nchan publishers to exit if no listeners#2326
Conversation
WalkthroughA set of nchan-related scripts were refactored to centralize publishing and abort logic: a Docker load script was rewritten from Bash to PHP CLI; Changes
Sequence Diagram(s)sequenceDiagram
participant PHP_Script
participant DockerEngine
participant PublishLib
participant NChan
participant PID_File
loop Forever
PHP_Script->>DockerEngine: shell_exec("docker stats ...")
DockerEngine-->>PHP_Script: stats output (or null)
PHP_Script->>PublishLib: publish_md5(endpoint, output, opt..., abort=true, abortTime)
PublishLib->>NChan: POST message if changed (or manage dedupe)
alt No subscribers > abortTime
PublishLib->>PID_File: remove entry (removeNChanScript)
PublishLib->>PHP_Script: exit process
end
alt output == null
PHP_Script->>PHP_Script: sleep 10s
else
PHP_Script->>PHP_Script: sleep 1s
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Squidly271
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
emhttp/plugins/dynamix/include/publish.php (1)
58-66: Use curl_getinfo() for HTTP status instead of regex scrapingRelying on digits in curl_error() is brittle. Capture the HTTP response code via curl_getinfo before closing the handle and use that for decisions (500/507).
- curl_close($com[$endpoint]); - unset($com[$endpoint]); - - preg_match_all("/[0-9]+/",$err,$matches); + $http = curl_getinfo($com[$endpoint], CURLINFO_RESPONSE_CODE); + curl_close($com[$endpoint]); + unset($com[$endpoint]);- if ( ($matches[0][0] ?? "") == 507 || ($matches[0][0] ?? "") == 500 ) { + if ($http === 507 || $http === 500) {
🧹 Nitpick comments (3)
emhttp/plugins/dynamix/include/publish.php (3)
28-31: Fix typos in new doc comments
- “enpoints” → “endpoints”
- “sript” → “script”
-// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other enpoints succeed. -// If this is a problem, don't use $abort and instead handle this in the script or utilize a single sript per endpoint. +// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other endpoints succeed. +// If this is a problem, don't use $abort and instead handle this in the script or utilize a single script per endpoint.
73-75: Minor: log context is good; consider consolidating error pathYou already log curl_error() above with endpoint. This second log on false reply is fine, but if you prefer less noise, guard to avoid duplicate logs for the same failure.
- if ($reply===false) - my_logger("curl to $endpoint failed", 'publish'); + if ($reply === false && !$err) { + my_logger("curl to $endpoint failed", 'publish'); + }
76-91: Avoid exiting from a library function in non-CLI contextsExiting inside publish() can be surprising if $abort ever gets enabled outside CLI. Guard exit() by SAPI, and return false otherwise. Also, return an explicit 0 exit code for the normal timeout case.
- my_logger("$endpoint timed out after $abortTime seconds. Exiting.", 'publish'); - - removeNChanScript(); - exit(); + my_logger("$endpoint timed out after $abortTime seconds. Exiting.", 'publish'); + removeNChanScript(); + if (PHP_SAPI === 'cli') { + exit(0); + } + return false;Please confirm that publish($abort=true) is only used by CLI scripts (as in docker_load). If there are any web/GUI code paths, the guard above prevents unexpected request termination.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load(1 hunks)emhttp/plugins/dynamix/include/publish.php(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-03-27T22:04:00.594Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php:58-74
Timestamp: 2025-03-27T22:04:00.594Z
Learning: The file `emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php` is synced from a different repository, and modifications should not be suggested in this repository's context. Changes should be proposed in the source repository instead.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-07-11T17:24:33.977Z
Learnt from: Squidly271
PR: unraid/webgui#2293
File: emhttp/plugins/dynamix.docker.manager/include/Helpers.php:302-304
Timestamp: 2025-07-11T17:24:33.977Z
Learning: In the Unraid WebGUI Docker manager (emhttp/plugins/dynamix.docker.manager/include/Helpers.php), the xmlSecurity() function call is intentionally commented out to allow users to include < and > characters in their Docker template variables on their own servers. Default templates still explicitly disallow tags, but users can customize templates as needed in their self-hosted environment.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-03-27T22:04:34.550Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/web-components-extractor.php:13-19
Timestamp: 2025-03-27T22:04:34.550Z
Learning: The file emhttp/plugins/dynamix.my.servers/include/web-components-extractor.php is synced from another repository and should not be modified directly in the webgui repository.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-02-27T21:53:37.589Z
Learnt from: dlandon
PR: unraid/webgui#2035
File: etc/rc.d/rc.rsyslogd:48-50
Timestamp: 2025-02-27T21:53:37.589Z
Learning: For the unraid/webgui project, simple error handling is preferred over extensive error checking in shell scripts. The copy operation for configuration files like in rc.rsyslogd doesn't require additional error logging, permission checks, or return statements.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-02-27T21:50:34.913Z
Learnt from: dlandon
PR: unraid/webgui#2035
File: emhttp/plugins/dynamix/scripts/syslogfilter:45-48
Timestamp: 2025-02-27T21:50:34.913Z
Learning: The syslog filter script in the unraid/webgui repository does not require explicit error handling for the rsyslog service restart. The simple approach without additional error checking is sufficient for this implementation.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-02-27T21:52:42.541Z
Learnt from: dlandon
PR: unraid/webgui#2035
File: emhttp/plugins/dynamix/SyslogFilter.page:19-26
Timestamp: 2025-02-27T21:52:42.541Z
Learning: In the unraid/webgui repository, basic error handling is sufficient for file operations like checking file existence and using the @ operator to suppress errors, without additional error messages or notifications.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
🔇 Additional comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
27-29: LGTM: abort behavior wired correctlypublish("dockerload", ..., true, 120) aligns with the new abort logic and will exit cleanly after 120s of no subscribers.
Squidly271
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/nchan/update_1 (1)
128-128: Optional: avoid magic literals; improve readabilityUse named variables to document intent and reduce ambiguity:
- $md5_old = publish('update1',$echo,1,true)!==false ? $md5_new : -1; + // Abort after 1s if no listeners present. + $abortTimeSec = 1; + $abortIfNoListeners = true; + $md5_old = publish('update1', $echo, $abortTimeSec, $abortIfNoListeners) !== false ? $md5_new : -1;If PHP 8 is available and allowed in this codebase, consider named args instead:
$md5_old = publish('update1', $echo, abortTime: 1, abort: true) !== false ? $md5_new : -1;emhttp/plugins/dynamix/nchan/update_3 (1)
165-165: Optional: clarify intent by avoiding magic literalsMirror the readability improvement here:
- $md5_old = publish('update3',$echo,1,true)!==false ? $md5_new : -1; + $abortTimeSec = 1; + $abortIfNoListeners = true; + $md5_old = publish('update3', $echo, $abortTimeSec, $abortIfNoListeners) !== false ? $md5_new : -1;If allowed, named args make this self-documenting:
$md5_old = publish('update3', $echo, abortTime: 1, abort: true) !== false ? $md5_new : -1;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
emhttp/plugins/dynamix/nchan/update_1(1 hunks)emhttp/plugins/dynamix/nchan/update_2(1 hunks)emhttp/plugins/dynamix/nchan/update_3(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- emhttp/plugins/dynamix/nchan/update_2
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-03-27T22:04:00.594Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php:58-74
Timestamp: 2025-03-27T22:04:00.594Z
Learning: The file `emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php` is synced from a different repository, and modifications should not be suggested in this repository's context. Changes should be proposed in the source repository instead.
Applied to files:
emhttp/plugins/dynamix/nchan/update_1emhttp/plugins/dynamix/nchan/update_3
🔇 Additional comments (1)
emhttp/plugins/dynamix/nchan/update_3 (1)
165-165: Validate publish() parameter order is correct here tooAs with update_1, passing (1, true) presumes publish($channel, $payload, $abortTime, $abort). Please confirm it matches publish.php’s signature to ensure the abort-on-no-listener behavior triggers as intended.
You can reuse the verification script from the update_1 comment to check both the function definition and all call sites for consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
21-26: shell_exec null-check misses non-zero exit codes; use exec() to capture status and avoid publishing error textOn failure, shell_exec returns null only in some cases; commands can fail with non-zero exit but still return stderr text (due to
2>&1). That would be published to clients. Use exec to capture exit status and bail out cleanly.- $output = shell_exec("docker stats --no-stream --format='{{.ID}};{{.CPUPerc}};{{.MemUsage}}' 2>&1"); - if ($output === null) { - my_logger("docker stats failed"); - removeNChanScript(); - exit(1); - } + $lines = []; + $code = 0; + exec("docker stats --no-stream --format='{{.ID}};{{.CPUPerc}};{{.MemUsage}}' 2>&1", $lines, $code); + if ($code !== 0) { + my_logger("docker stats failed: ".implode("\n", $lines)); + removeNChanScript(); + exit(1); + } + $output = implode("\n", $lines);
🧹 Nitpick comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
20-30: Optional: Treat transient docker stats errors with retry/backoff instead of immediate exitA single hiccup will terminate the loop. Consider a limited retry with backoff before calling removeNChanScript()+exit to reduce churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-03-27T22:04:00.594Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php:58-74
Timestamp: 2025-03-27T22:04:00.594Z
Learning: The file `emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php` is synced from a different repository, and modifications should not be suggested in this repository's context. Changes should be proposed in the source repository instead.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-07-11T17:24:33.977Z
Learnt from: Squidly271
PR: unraid/webgui#2293
File: emhttp/plugins/dynamix.docker.manager/include/Helpers.php:302-304
Timestamp: 2025-07-11T17:24:33.977Z
Learning: In the Unraid WebGUI Docker manager (emhttp/plugins/dynamix.docker.manager/include/Helpers.php), the xmlSecurity() function call is intentionally commented out to allow users to include < and > characters in their Docker template variables on their own servers. Default templates still explicitly disallow tags, but users can customize templates as needed in their self-hosted environment.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-03-27T22:04:34.550Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/web-components-extractor.php:13-19
Timestamp: 2025-03-27T22:04:34.550Z
Learning: The file emhttp/plugins/dynamix.my.servers/include/web-components-extractor.php is synced from another repository and should not be modified directly in the webgui repository.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-02-27T21:53:37.589Z
Learnt from: dlandon
PR: unraid/webgui#2035
File: etc/rc.d/rc.rsyslogd:48-50
Timestamp: 2025-02-27T21:53:37.589Z
Learning: For the unraid/webgui project, simple error handling is preferred over extensive error checking in shell scripts. The copy operation for configuration files like in rc.rsyslogd doesn't require additional error logging, permission checks, or return statements.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-02-27T21:50:34.913Z
Learnt from: dlandon
PR: unraid/webgui#2035
File: emhttp/plugins/dynamix/scripts/syslogfilter:45-48
Timestamp: 2025-02-27T21:50:34.913Z
Learning: The syslog filter script in the unraid/webgui repository does not require explicit error handling for the rsyslog service restart. The simple approach without additional error checking is sufficient for this implementation.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
📚 Learning: 2025-02-27T21:52:42.541Z
Learnt from: dlandon
PR: unraid/webgui#2035
File: emhttp/plugins/dynamix/SyslogFilter.page:19-26
Timestamp: 2025-02-27T21:52:42.541Z
Learning: In the unraid/webgui repository, basic error handling is sufficient for file operations like checking file existence and using the @ operator to suppress errors, without additional error messages or notifications.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
🔇 Additional comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
27-28: Comment/code mismatch on “120 seconds” vs parameters (1, true); confirm publish() semantics and alignThe comment says “exit … after 120 seconds” but the call passes
(1, true). If the third param is a timeout (seconds), use 120; if it’s a min-listener threshold, update the comment. Also confirm whether publish() will actually exit this script on timeout as implied.Option A (if 3rd param is timeout in seconds):
- // publish and exit the script if no listeners after 120 seconds - publish("dockerload", $output, 1, true); + // publish and exit the script if no listeners after 120 seconds + publish("dockerload", $output, 120, true);Option B (if 3rd param is listener threshold):
- // publish and exit the script if no listeners after 120 seconds + // publish; abort if fewer than 1 listeners (see publish() semantics) publish("dockerload", $output, 1, true);Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load(1 hunks)emhttp/plugins/dynamix.vm.manager/nchan/vm_usage(1 hunks)emhttp/plugins/dynamix/include/publish.php(5 hunks)emhttp/plugins/dynamix/nchan/update_1(1 hunks)emhttp/plugins/dynamix/nchan/update_2(1 hunks)emhttp/plugins/dynamix/nchan/update_3(1 hunks)emhttp/plugins/dynamix/nchan/ups_status(1 hunks)emhttp/plugins/dynamix/nchan/wg_poller(1 hunks)emhttp/plugins/dynamix/nchan/wlan0(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- emhttp/plugins/dynamix/nchan/wg_poller
🚧 Files skipped from review as they are similar to previous changes (5)
- emhttp/plugins/dynamix/nchan/update_1
- emhttp/plugins/dynamix/nchan/update_2
- emhttp/plugins/dynamix/nchan/update_3
- emhttp/plugins/dynamix.docker.manager/nchan/docker_load
- emhttp/plugins/dynamix/include/publish.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
🔇 Additional comments (1)
emhttp/plugins/dynamix/nchan/wlan0 (1)
68-70: LGTM — explicit buffer length and abort flag are clearUsing buffer length 1 with abort=true is clear and matches the intended behavior described in the comment. No further changes needed here.
If publish() supports an abort timeout, confirm the default grace period is appropriate for WiFi toggling to avoid premature exit on transient subscriber drops.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
2-2: Use a single long PHP open tag and remove short open/close togglesShort open tags can be disabled in CLI and will break execution. Keep one
<?phpat the top and do not close/re-open.-<?PHP +<?php @@ -?> -<?Also applies to: 13-14
🧹 Nitpick comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
1-1: Nit: Prefer env in shebang for portabilityNot required for Unraid, but
envimproves portability across environments.-#!/usr/bin/php -q +#!/usr/bin/env php -q
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-05-31T05:10:13.120Z
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
keep publishing at a slower rate if no containers are running. Precursor to having publish not publish if md5 hasn't changed but still honoring timeouts.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (2)
2-2: Use a single long PHP open tag; remove short tags and block togglingShort tags can be disabled and will break CLI. Keep one
<?phpat the top and don’t close/reopen the PHP block.-<?PHP +<?php @@ -?> -<? +// keep a single PHP block; no need to close/reopenAlso applies to: 13-14
20-30: Publish after validating docker stats; detect empty output; back off and trigger exit correctlyCurrent loop publishes unconditionally and only treats
nullas “no stats” (common case is empty string). It can spam Nchan with empty/error payloads and may never hit the intended exit pattern.-while (true) { - $output = shell_exec("docker stats --no-stream --format='{{.ID}};{{.CPUPerc}};{{.MemUsage}}' 2>&1"); - - publish("dockerload", $output,true); - if ( $output === null ) { - // slow down publishing if no containers are running - sleep(10); - } else { - sleep(1); - } -} +while (true) { + // Capture exit status and output (stderr included) + $lines = []; + $code = 0; + exec("docker stats --no-stream --format='{{.ID}};{{.CPUPerc}};{{.MemUsage}}' 2>&1", $lines, $code); + $output = implode("\n", $lines); + $hasStats = ($code === 0) && (trim($output) !== ''); + + if ($hasStats) { + // Normal path: publish validated stats once per second + $noDockerStats = false; + $noDockerStatsTime = null; + publish("dockerload", $output, true, $exitAfterTimeout); + sleep(1); + continue; + } + + // No stats (e.g., no containers) or docker stats failed. + // Publish once to establish the channel and start the abort timer, + // then back off; after the timeout, allow a second publish to trigger exit-if-no-listeners. + if (!$noDockerStats) { + publish("dockerload", "", true, $exitAfterTimeout); + $noDockerStats = true; + $noDockerStatsTime = time(); + } + + if ($noDockerStatsTime !== null && (time() - $noDockerStatsTime) > $exitAfterTimeout) { + // Allow next loop to publish again (second attempt) to trigger exit + $noDockerStats = false; + $noDockerStatsTime = null; + } + + sleep(10); +}Notes:
- Avoid publishing raw error text to the channel; treat non-zero exit as “no stats” and back off.
- This aligns with the “second publish after timeout” requirement to exit when there are no listeners.
🧹 Nitpick comments (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (1)
1-1: Nit: -q is unnecessary for CLI shebang
-qsuppresses CGI headers; it’s redundant for CLI. Consider dropping for clarity.-#!/usr/bin/php -q +#!/usr/bin/php
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-05-31T05:10:13.120Z
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_load
Keep track of the md5 within a separate function. Avoids duplication of code on the nchan scripts, and allows for the script to still exit if no listeners without adding in more code within each script to handle this.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
emhttp/plugins/dynamix/include/publish.php (1)
81-86: Use CURLINFO_RESPONSE_CODE instead of regex-parsing curl_errorParsing digits from curl_error is brittle. Rely on curl_getinfo status codes.
- preg_match_all("/[0-9]+/",$err,$matches); - // 500: out of shared memory when creating a channel - // 507: out of shared memory publishing a message - - if ( ($matches[0][0] ?? "") == 507 || ($matches[0][0] ?? "") == 500 ) { + // 500: out of shared memory creating a channel + // 507: out of shared memory publishing a message + $status = curl_getinfo($com[$endpoint], CURLINFO_RESPONSE_CODE); + if ($status === 507 || $status === 500) { my_logger("Nchan out of shared memory. Reloading nginx");
♻️ Duplicate comments (3)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load (3)
2-2: Avoid short open tags and toggling; use a single<?phpblockKeep one long tag at the top; remove the close/reopen.
-<?PHP +<?php @@ -?> -<? +// keep a single PHP block; no need to close/reopenAlso applies to: 13-14
17-19: Initialize no-stats state and timeout controlsPrevents spamming publishes when no containers and supports timed exit-on-no-listeners.
require_once "$docroot/webGui/include/Helpers.php"; require_once "$docroot/webGui/include/publish.php"; +// Controls for no-stats backoff and exit-if-no-listeners +$noDockerStats = false; +$noDockerStatsTime = null; +$exitAfterTimeout = 15; // seconds; tune as needed
20-30: Use exec() with exit status; treat empty output as “no stats”; publish after validationshell_exec returns null on failure, but empty output (common when no containers) is not null. This can lead to unintended behavior. Also, publish after confirming you have data or once per backoff cycle for no-stats.
-while (true) { - $output = shell_exec("docker stats --no-stream --format='{{.ID}};{{.CPUPerc}};{{.MemUsage}}' 2>&1"); - - publish_md5("dockerload", $output,true); - if ( $output === null ) { - // slow down publishing if no containers are running - sleep(10); - } else { - sleep(1); - } -} +while (true) { + // Capture exit status and output + $lines = []; + $code = 0; + exec("docker stats --no-stream --format='{{.ID}};{{.CPUPerc}};{{.MemUsage}}' 2>&1", $lines, $code); + $output = implode("\n", $lines); + $hasStats = ($code === 0) && (trim($output) !== ''); + + if ($hasStats) { + // Normal path: publish stats once per second + $noDockerStats = false; + $noDockerStatsTime = null; + publish_md5("dockerload", $output, true); + sleep(1); + continue; + } + + // No stats: publish once, then back off; allow second publish after timeout to trigger exit-if-no-listeners + if (!$noDockerStats) { + publish_md5("dockerload", "", true, $exitAfterTimeout); + $noDockerStats = true; + $noDockerStatsTime = time(); + } + if ($noDockerStatsTime !== null && (time() - $noDockerStatsTime) > $exitAfterTimeout) { + $noDockerStats = false; // allow a second publish on next loop to trigger exit + $noDockerStatsTime = null; + } + sleep(10); +}
🧹 Nitpick comments (3)
emhttp/plugins/dynamix/include/publish.php (2)
28-36: Fix typos and clarify overload docs (opt1/opt2/opt3) for maintainabilityMinor typos and duplicate sentence; propose clearer wording.
-// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other enpoints succeed. -// If this is a problem, don't use $abort and instead handle this in the script or utilize a single sript per endpoint. +// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other endpoints succeed. +// If this is a problem, don't use $abort. Handle it in the script or use a single script per endpoint. // -// $opt1: length of the buffer. If not numeric, it's a boolean for $abort. If not numeric, it's a boolean for $abort. -// $opt2: if $opt1 is not numeric, it's a boolean for $abort. -// $opt3: if $opt1 is not numeric, it's a value for $abortTime. +// $opt1: If numeric, buffer_length; if boolean, $abort. +// $opt2: If $opt1 is numeric, $abort; otherwise $abortTime. +// $opt3: If $opt1 is numeric, $abortTime; otherwise ignored unless $opt2 is falsy (fallback).
12-16: Unify PHP tags (optional) — keep a single long<?phpblockShort tags and open/close toggles can be fragile under different SAPIs.
-?> -<? +// keep a single PHP block; no need to close/reopenemhttp/plugins/dynamix/nchan/update_3 (1)
162-163: LGTM: Consolidation to publish_md5 with abort is consistent and correctArray payload is JSON-encoded internally; boolean overload engages abort behavior with default timeout.
If desired to shorten exit window:
publish_md5('update3', $echo, true, 30);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
emhttp/plugins/dynamix.docker.manager/nchan/docker_load(1 hunks)emhttp/plugins/dynamix.vm.manager/nchan/vm_usage(1 hunks)emhttp/plugins/dynamix/include/publish.php(5 hunks)emhttp/plugins/dynamix/nchan/update_1(1 hunks)emhttp/plugins/dynamix/nchan/update_2(1 hunks)emhttp/plugins/dynamix/nchan/update_3(1 hunks)emhttp/plugins/dynamix/nchan/ups_status(1 hunks)emhttp/plugins/dynamix/nchan/wg_poller(1 hunks)emhttp/plugins/dynamix/nchan/wlan0(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- emhttp/plugins/dynamix/nchan/update_1
- emhttp/plugins/dynamix/nchan/wg_poller
- emhttp/plugins/dynamix/nchan/ups_status
- emhttp/plugins/dynamix.vm.manager/nchan/vm_usage
- emhttp/plugins/dynamix/nchan/wlan0
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix.vm.manager/nchan/vm_usage:93-93
Timestamp: 2025-08-10T20:48:52.063Z
Learning: In the unraid/webgui codebase, Squidly271 prefers the publish() function to internally handle parameter type overloading (e.g., accepting boolean true in place of buffer length 1) to avoid repetitive parameter specifications and maintain backwards compatibility. The publish() function in emhttp/plugins/dynamix/include/publish.php is designed to sort out these parameters internally rather than requiring explicit values at every call site.
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-05-31T05:10:13.120Z
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_loademhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-08-10T20:48:52.063Z
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix.vm.manager/nchan/vm_usage:93-93
Timestamp: 2025-08-10T20:48:52.063Z
Learning: In the unraid/webgui codebase, Squidly271 prefers the publish() function to internally handle parameter type overloading (e.g., accepting boolean true in place of buffer length 1) to avoid repetitive parameter specifications and maintain backwards compatibility. The publish() function in emhttp/plugins/dynamix/include/publish.php is designed to sort out these parameters internally rather than requiring explicit values at every call site.
Applied to files:
emhttp/plugins/dynamix.docker.manager/nchan/docker_loademhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-03-27T22:04:00.594Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php:58-74
Timestamp: 2025-03-27T22:04:00.594Z
Learning: The file `emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php` is synced from a different repository, and modifications should not be suggested in this repository's context. Changes should be proposed in the source repository instead.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
🧬 Code Graph Analysis (1)
emhttp/plugins/dynamix/include/publish.php (1)
emhttp/plugins/dynamix/include/Wrappers.php (2)
my_logger(220-222)file_put_contents_atomic(25-40)
🔇 Additional comments (2)
emhttp/plugins/dynamix/include/publish.php (1)
46-55: Good: Backward-compatible parameter overloading is clean and aligns with call-site simplicityThe opt1/opt2/opt3 handling preserves legacy calls and supports boolean overload for $abort with default timeout, matching the preferred pattern in this codebase.
emhttp/plugins/dynamix/nchan/update_2 (1)
502-503: LGTM: Centralized dedupe + abort via publish_md5 simplifies the loopPassing true leverages the boolean overload for abort with default timeout and internal JSON handling of arrays. Matches the preferred pattern.
If you want faster exit when no listeners, consider a custom timeout:
publish_md5('update2', $echo, true, 30); // exit-around ~60s (two publish attempts)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
emhttp/plugins/dynamix/include/publish.php (2)
42-44: LGTM: Corrected json_encode flagsRemoving the unintended
trueflags fixes the JSON_HEX_TAG misconfiguration. Matches prior guidance.Also applies to: 128-130
37-37: LGTM: Initialize static arraysStatic $abortStart/$com/$lens are properly initialized, avoiding scalar-to-array errors on write.
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/include/publish.php (2)
31-36: Fix typos and align option docs with implementationCurrent comments have typos and misstate $opt2’s role when $opt1 is non-numeric.
-// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other enpoints succeed. -// If this is a problem, don't use $abort and instead handle this in the script or utilize a single sript per endpoint. -// -// $opt1: length of the buffer. If not numeric, it's a boolean for $abort. If not numeric, it's a boolean for $abort. -// -// $opt2: if $opt1 is not numeric, it's a boolean for $abort. -// -// $opt3: if $opt1 is not numeric, it's a value for $abortTime. +// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other endpoints succeed. +// If this is a problem, don't use $abort and instead handle this in the script or utilize a single script per endpoint. +// +// $opt1: buffer length (numeric). If not numeric, it's treated as a boolean for $abort. +// $opt2: when $opt1 is non-numeric, this is $abortTime (seconds). +// $opt3: when $opt1 is numeric, this is $abortTime (seconds) [default 120].
75-86: Optionally log HTTP status and branch on code instead of parsing error textMore robust than regex parsing of curl_error; also improves diagnostics.
- $err = curl_error($com[$endpoint]); + $err = curl_error($com[$endpoint]); + $http_code = curl_getinfo($com[$endpoint], CURLINFO_RESPONSE_CODE); if ($err) { - my_logger("curl error: $err endpoint: $endpoint"); + my_logger("curl error: $err (HTTP $http_code) endpoint: $endpoint"); curl_close($com[$endpoint]); unset($com[$endpoint]); - preg_match_all("/[0-9]+/",$err,$matches); + preg_match_all("/[0-9]+/",$err,$matches); // 500: out of shared memory when creating a channel // 507: out of shared memory publishing a message - if ( ($matches[0][0] ?? "") == 507 || ($matches[0][0] ?? "") == 500 ) { + if ($http_code === 507 || $http_code === 500) { my_logger("Nchan out of shared memory. Reloading nginx");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/publish.php(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix.vm.manager/nchan/vm_usage:93-93
Timestamp: 2025-08-10T20:48:52.063Z
Learning: In the unraid/webgui codebase, Squidly271 prefers the publish() function to internally handle parameter type overloading (e.g., accepting boolean true in place of buffer length 1) to avoid repetitive parameter specifications and maintain backwards compatibility. The publish() function in emhttp/plugins/dynamix/include/publish.php is designed to sort out these parameters internally rather than requiring explicit values at every call site.
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-08-10T20:48:52.063Z
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix.vm.manager/nchan/vm_usage:93-93
Timestamp: 2025-08-10T20:48:52.063Z
Learning: In the unraid/webgui codebase, Squidly271 prefers the publish() function to internally handle parameter type overloading (e.g., accepting boolean true in place of buffer length 1) to avoid repetitive parameter specifications and maintain backwards compatibility. The publish() function in emhttp/plugins/dynamix/include/publish.php is designed to sort out these parameters internally rather than requiring explicit values at every call site.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-03-27T22:04:00.594Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php:58-74
Timestamp: 2025-03-27T22:04:00.594Z
Learning: The file `emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php` is synced from a different repository, and modifications should not be suggested in this repository's context. Changes should be proposed in the source repository instead.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-05-31T05:10:13.120Z
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
🧬 Code Graph Analysis (1)
emhttp/plugins/dynamix/include/publish.php (1)
emhttp/plugins/dynamix/include/Wrappers.php (2)
my_logger(220-222)file_put_contents_atomic(25-40)
🔇 Additional comments (3)
emhttp/plugins/dynamix/include/publish.php (3)
57-61: LGTM: Reset curl handle on buffer_length changeClosing and recreating the handle when buffer_length changes avoids mismatched connections and correctly clears abort timing.
96-111: LGTM: Abort-on-no-subscribers mechanismClean handling of first-detect timer, reset on subscriber presence, and script cleanup before exit. Aligns with PR objective.
132-146: LGTM: md5-based dedup with periodic republish when abortingCorrectly avoids redundant publishes, with time-based republish to re-check subscriber presence and trigger exit if needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/nchan/vm_dashusage (2)
43-45: Graceful cleanup path looks good; prefer explicit success exit codeCentralizing PID cleanup via removeNChanScript() is a solid improvement. Minor nit: use an explicit success code for clarity.
- exit; + exit(0);Also applies to: 48-50
24-24: Remove leftover md5_old; it’s unused after migrating to publish_md5Dead variable from prior manual MD5 flow.
-$md5_old = -1;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/nchan/vm_dashusage(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix.vm.manager/nchan/vm_usage:93-93
Timestamp: 2025-08-10T20:48:52.063Z
Learning: In the unraid/webgui codebase, Squidly271 prefers the publish() function to internally handle parameter type overloading (e.g., accepting boolean true in place of buffer length 1) to avoid repetitive parameter specifications and maintain backwards compatibility. The publish() function in emhttp/plugins/dynamix/include/publish.php is designed to sort out these parameters internally rather than requiring explicit values at every call site.
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
🔇 Additional comments (1)
emhttp/plugins/dynamix/nchan/vm_dashusage (1)
112-113: publish_md5() already exits when no listeners for vm_dashusage – no change neededVerified that calling
publish_md5('vm_dashusage', $echo, true);passes
trueas the$abortflag (third parameter), sopublish_md5()will check for subscribers and exit if none are listening. No additional arguments or call-site changes are required.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
emhttp/plugins/dynamix/include/publish.php (3)
37-37: Resolved: initialize statics to arraysInitializing $abortStart and $com eliminates “Cannot use a scalar value as an array” issues. Thanks for addressing this.
42-44: json_encode flags corrected — LGTMRemoving the boolean “true” from json_encode prevents unintended flag behavior. Matches earlier suggestion.
Also applies to: 128-130
153-157: Harden script name detection for non-CLI SAPI (future)Current usage is CLI-only and works as-is. For future web contexts, prefer SCRIPT_FILENAME/PHP_SELF with PHP_SAPI.
- $script = trim(str_replace("$docroot/","",(php_sapi_name() === 'cli') ? $argv[0] : $_SERVER['argv'][0])); + $source = (PHP_SAPI === 'cli' && isset($argv[0])) + ? $argv[0] + : ($_SERVER['SCRIPT_FILENAME'] ?? $_SERVER['PHP_SELF'] ?? ''); + $script = trim(str_replace("$docroot/", "", $source));
🧹 Nitpick comments (3)
emhttp/plugins/dynamix/include/publish.php (3)
28-36: Tighten option docs: fix typos/duplication and note unsupported 0s timeoutsMinor comment cleanups and an explicit note will reduce confusion at call sites.
-// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other enpoints succeed. +// If a script publishes to multiple endpoints, timing out on one endpoint will terminate the entire script even if other endpoints succeed. -// If this is a problem, don't use $abort and instead handle this in the script or utilize a single sript per endpoint. +// If this is a problem, don't use $abort; instead handle this in the script or run a single script per endpoint. -// $opt1: length of the buffer. If not numeric, it's a boolean for $abort. If not numeric, it's a boolean for $abort. +// $opt1: length of the buffer. If not numeric, it's a boolean for $abort. -// $opt2: if $opt1 is not numeric, it's a boolean for $abort. +// $opt2: if $opt1 is not numeric, it's a boolean for $abort. -// $opt3: if $opt1 is not numeric, it's a value for $abortTime. +// $opt3: if $opt1 is not numeric, it's the value for $abortTime (seconds). +// Note: 0-second timeouts are not supported; minimum valid timeout is > 0 seconds. function publish($endpoint, $message, $opt1=1, $opt2=false, $opt3=120) {
65-71: Prefer “application/json” for Accept headerUse the standard media type.
- CURLOPT_HTTPHEADER => ['Accept:text/json'], + CURLOPT_HTTPHEADER => ['Accept: application/json'],
109-109: Optional: unset instead of null for timer resetUsing unset makes the intent explicit and avoids any implicit null arithmetic if this block ever changes.
- $abortStart[$endpoint] = null; + unset($abortStart[$endpoint]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/publish.php(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-08-10T20:48:52.063Z
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix.vm.manager/nchan/vm_usage:93-93
Timestamp: 2025-08-10T20:48:52.063Z
Learning: In the unraid/webgui codebase, Squidly271 prefers the publish() function to internally handle parameter type overloading (e.g., accepting boolean true in place of buffer length 1) to avoid repetitive parameter specifications and maintain backwards compatibility. The publish() function in emhttp/plugins/dynamix/include/publish.php is designed to sort out these parameters internally rather than requiring explicit values at every call site.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-03-27T22:04:00.594Z
Learnt from: zackspear
PR: unraid/webgui#2099
File: emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php:58-74
Timestamp: 2025-03-27T22:04:00.594Z
Learning: The file `emhttp/plugins/dynamix.my.servers/include/activation-code-extractor.php` is synced from a different repository, and modifications should not be suggested in this repository's context. Changes should be proposed in the source repository instead.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-05-31T05:10:13.120Z
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
📚 Learning: 2025-08-10T23:33:40.293Z
Learnt from: Squidly271
PR: unraid/webgui#2326
File: emhttp/plugins/dynamix/include/publish.php:46-55
Timestamp: 2025-08-10T23:33:40.293Z
Learning: In the unraid/webgui codebase, the publish() and publish_md5() functions in emhttp/plugins/dynamix/include/publish.php deliberately do not support 0-second timeouts for the abort mechanism. The use of the elvis operator (?:) for timeout parameters is intentional, as timeout values of 0 are not relevant and not supported. The minimum valid timeout should be greater than 0 seconds.
Applied to files:
emhttp/plugins/dynamix/include/publish.php
🧬 Code Graph Analysis (1)
emhttp/plugins/dynamix/include/publish.php (1)
emhttp/plugins/dynamix/include/Wrappers.php (2)
my_logger(220-222)file_put_contents_atomic(25-40)
🔇 Additional comments (5)
emhttp/plugins/dynamix/include/publish.php (5)
14-15: LGTM: Safe docroot default under CLISwitching to null-coalescing avoids CLI notices when DOCUMENT_ROOT isn’t set. Good fix.
46-55: Back-compat param overloading and timeout semantics — LGTMInterpretation of opt1/opt2/opt3 keeps legacy calls working and (intentionally) does not support 0s timeouts per team preference.
Also applies to: 120-126
58-61: Good: reset curl handle and abort timer on buffer length changeClosing/unsetting the handle and clearing the timer avoids stale state across buffer_length changes.
77-91: Error handling + guarded nginx restart — LGTMDetailed logging, handle reset, and single-restart gating via /tmp/publishPaused are sound.
96-111: Abort-on-no-subscribers flow and cleanup — LGTMDecoding reply, tracking per-endpoint timers, pid cleanup, and exit are consistent with the PR goal.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
zackspear
left a comment
zackspear
left a comment
There was a problem hiding this comment.
My comment on publish.php would result in these other files needing updating as well. Curious about our thoughts on the suggestions.
4 participants
Summary by CodeRabbit
Refactor
New Features
Chores