Skip to content

Conversation

@daniel-santos
Copy link
Contributor

Checklist

  • [no] I have read the OpenWISP Contributing Guidelines.
  • [yes] I have manually tested the changes proposed in this pull request.
  • [n/a] I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • [n/a] I have updated the documentation.

Reference to Existing Issue

Closes #159

Description of Changes

Uses --data's special treatment of @ to load data from file instead of in-lining it on the command line.

@nemesifier
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

The change modifies the data transmission mechanism in the monitoring agent script. Instead of reading file contents into an in-memory variable using cat and passing it via the -d parameter, the script now uses curl's --data @filename`` syntax to send the file directly from disk. The script additionally checks for file existence and gracefully skips transmission if the file is missing, rather than failing on a missing file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing a command-line length issue by avoiding inline data passing.
Description check ✅ Passed The description covers the key aspects including manual testing confirmation and a clear explanation of the technical change made, though it lacks detail in some checklist items.
Linked Issues check ✅ Passed The code changes directly address issue #159 by replacing inline JSON command-line data with file-based curl transmission using --data @$filename.
Out of Scope Changes check ✅ Passed All changes are focused on the specific objective of addressing the shell command-line length limit issue; no extraneous modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d631074 and f6efa39.

📒 Files selected for processing (1)
  • openwisp-monitoring/files/monitoring.agent
🔇 Additional comments (2)
openwisp-monitoring/files/monitoring.agent (2)

154-158: Good defensive check for file existence after decompression.

This correctly handles the edge case where the decompressed file might not exist (e.g., if gzip -d failed silently or the file was removed between operations). The continue gracefully skips to the next file.


177-177: Core fix correctly addresses the command-line length limitation.

Using -d "@$filename" makes curl read the payload directly from the file, bypassing shell expansion and command-line length limits. This is the idiomatic solution for large payloads.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nemesifier nemesifier merged commit 06be32d into openwisp:master Jan 22, 2026
3 checks passed
@nemesifier nemesifier changed the title Fix cmd line too long [fix] Fixed cmd line too long Jan 22, 2026
@nemesifier nemesifier added the bug Something isn't working label Jan 22, 2026
@github-project-automation github-project-automation bot moved this from To do (general) to Done in OpenWISP Contributor's Board Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Development

Successfully merging this pull request may close these issues.

[bug] transmission fails when message size exceeds shell limit

2 participants