feat: support Android apps in worker heartbeat and UI#14
Merged
Conversation
- Accept top-level `apps` array in heartbeat (alongside `containers`) - Store apps in new `apps` column on workers table (auto-migrated) - Convert Android app statuses to service entries for the dashboard - Fleet summary counts apps for Android workers - Fleet UI shows "Android" badge and renders apps distinctly - Dashboard disables Docker actions for Android instances
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds Android worker support: database gains an Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/main.py (1)
1562-1571: Consider extracting duplicate JSON parsing logic.The fleet summary duplicates the JSON parsing and count logic that
_parse_worker_json()already handles. Consider reusing it:Suggested refactor
for w in workers: - sys_info = json.loads(w.get("system_info", "{}")) - is_android = sys_info.get("device_type") == "android" - if is_android: - apps = json.loads(w.get("apps", "[]")) - total_containers += len(apps) - total_running += sum(1 for a in apps if a.get("running")) - else: - containers = json.loads(w.get("containers", "[]")) - total_containers += len(containers) - total_running += sum(1 for c in containers if c.get("status") == "running") + _parse_worker_json(w) + total_containers += w["container_count"] + total_running += w["running_count"] if w["status"] == "online": online_workers += 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/main.py` around lines 1562 - 1571, The code duplicates JSON parsing and counting for workers; instead call the existing helper _parse_worker_json(w) (or adapt it to return both a list of containers/apps and the running count) and use its outputs to increment total_containers and total_running; replace the sys_info/json.loads blocks and the is_android branch with a single call to _parse_worker_json(w) and then do total_containers += len(containers) and total_running += running_count (or derive running_count from the returned containers), ensuring _parse_worker_json handles both "apps" and "containers" and returns a consistent structure.app/database.py (1)
181-185: Consider logging when migration adds the new column.The migration silently adds the
appscolumn. Adding a log statement would help operators understand schema changes during upgrades.Suggested improvement
if "apps" not in cols: await db.execute("ALTER TABLE workers ADD COLUMN apps TEXT NOT NULL DEFAULT '[]'") + _logger.info("Migrated workers table: added 'apps' column")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/database.py` around lines 181 - 185, The migration silently adds the apps column; update the block that checks PRAGMA table_info(workers) and executes ALTER TABLE workers ADD COLUMN apps ... to emit a log entry when the column is added. Use the project's existing logger (e.g., logger or process_logger) and log at info/debug level with a clear message like "Added 'apps' column to workers table during migration" and include the default value and SQL executed; keep the log right after the await db.execute(...) that adds the column so operators can see schema changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/database.py`:
- Around line 181-185: The migration silently adds the apps column; update the
block that checks PRAGMA table_info(workers) and executes ALTER TABLE workers
ADD COLUMN apps ... to emit a log entry when the column is added. Use the
project's existing logger (e.g., logger or process_logger) and log at info/debug
level with a clear message like "Added 'apps' column to workers table during
migration" and include the default value and SQL executed; keep the log right
after the await db.execute(...) that adds the column so operators can see schema
changes.
In `@app/main.py`:
- Around line 1562-1571: The code duplicates JSON parsing and counting for
workers; instead call the existing helper _parse_worker_json(w) (or adapt it to
return both a list of containers/apps and the running count) and use its outputs
to increment total_containers and total_running; replace the sys_info/json.loads
blocks and the is_android branch with a single call to _parse_worker_json(w) and
then do total_containers += len(containers) and total_running += running_count
(or derive running_count from the returned containers), ensuring
_parse_worker_json handles both "apps" and "containers" and returns a consistent
structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33aa87db-04d6-478f-b0cf-6fed8bffb861
⛔ Files ignored due to path filters (3)
app/static/favicon.svgis excluded by!**/*.svgdocs/icon.svgis excluded by!**/*.svgdocs/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
app/database.pyapp/main.pyapp/static/js/app.jsapp/templates/fleet.html
- Resilient JSON parsing: _safe_json() helper so one malformed DB row degrades that worker instead of 500-ing the whole fleet/services UI - Fleet summary now filters offline workers (was counting stale data) - Fleet UI: "Containers" label → "Services", onboarding text mentions Android app, app pills show 24h net traffic on hover tooltip - Dashboard sub-rows show net_tx/rx for Android instances instead of CPU/memory placeholders
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/templates/fleet.html (1)
142-147: Inconsistency withfmtNetBytesin app.js — missing null guard.Line 143 returns
b + ' B'which would produce"undefined B"or"null B"if called with falsy non-zero values. The equivalent function inapp/static/js/app.js:77uses(b || 0) + ' B'.Currently safe because lines 156-157 pre-default values with
|| 0, but the function isn't robust standalone. Also consider extracting this to a shared utility to avoid duplication.Proposed fix
function fmtBytes(b) { - if (!b || b < 1024) return b + ' B'; + if (!b || b < 1024) return (b || 0) + ' B'; if (b < 1048576) return (b / 1024).toFixed(1) + ' KB'; if (b < 1073741824) return (b / 1048576).toFixed(1) + ' MB'; return (b / 1073741824).toFixed(2) + ' GB'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/templates/fleet.html` around lines 142 - 147, The fmtBytes function can produce "undefined B" or "null B" for falsy non-zero inputs; update fmtBytes to defensively default b to 0 (e.g., treat b as (b || 0)) at the start of the function so it mirrors fmtNetBytes behavior and never returns "undefined B", and then consider extracting fmtBytes (and fmtNetBytes) into a shared utility to remove duplication and keep formatting logic consistent across app/templates/fleet.html and app/static/js/app.js.app/static/js/app.js (1)
523-528: Consider adding a visual indicator that CPU/Memory columns show network metrics for Android.The table headers remain "CPU" and "Memory" (lines 342-343), but Android sub-rows display network TX/RX. Users may be confused seeing byte values in these columns. A small inline label or tooltip on the cells could clarify this.
Otherwise the logic is sound — Android instances correctly show network metrics while non-Android instances retain CPU/memory display.
Also applies to: 532-532, 538-539
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/static/js/app.js` around lines 523 - 528, The CPU/Memory columns show network metrics for Android instances (inst.is_android) but lack any visual cue; update the code that builds cpuCell and memCell so when inst.is_android you append a short inline indicator or tooltip (e.g., " (net TX)" / " (net RX)" or a span with title attribute) to the displayed fmtNetBytes value, and ensure the same change is applied where cpuCell/memCell are used (refer to variables cpuCell, memCell and the inst.is_android branch); keep the existing formatting for non-Android instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/static/js/app.js`:
- Around line 523-528: The CPU/Memory columns show network metrics for Android
instances (inst.is_android) but lack any visual cue; update the code that builds
cpuCell and memCell so when inst.is_android you append a short inline indicator
or tooltip (e.g., " (net TX)" / " (net RX)" or a span with title attribute) to
the displayed fmtNetBytes value, and ensure the same change is applied where
cpuCell/memCell are used (refer to variables cpuCell, memCell and the
inst.is_android branch); keep the existing formatting for non-Android instances.
In `@app/templates/fleet.html`:
- Around line 142-147: The fmtBytes function can produce "undefined B" or "null
B" for falsy non-zero inputs; update fmtBytes to defensively default b to 0
(e.g., treat b as (b || 0)) at the start of the function so it mirrors
fmtNetBytes behavior and never returns "undefined B", and then consider
extracting fmtBytes (and fmtNetBytes) into a shared utility to remove
duplication and keep formatting logic consistent across app/templates/fleet.html
and app/static/js/app.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de118c8a-2260-4e6f-a36a-a641bda51266
📒 Files selected for processing (3)
app/main.pyapp/static/js/app.jsapp/templates/fleet.html
🚧 Files skipped from review as they are similar to previous changes (1)
- app/main.py
Workers are now identified by client_id (UNIQUE) instead of name. Existing workers are migrated with client_id = name for backward compat. The heartbeat accepts an optional client_id field; when absent, name is used as fallback. This prevents two devices with the same display name from overwriting each other's state. Also adds 8 tests covering heartbeat identity, fleet summary (online-only filtering, Android app counting, malformed JSON resilience), and worker list (Android vs Docker counting).
- Migration now recreates the idx_workers_status index after table rebuild (was lost on upgraded installs) - Log migration with _logger.info for observability - Fleet summary reuses _parse_worker_json() instead of inline parsing - fmtBytes() null-safe for undefined/zero values
Show ↑/↓ prefixes so it's clear the CPU/Memory columns display network traffic for Android instances.
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.
Summary
appsarray in worker heartbeat payload (alongsidecontainers), matching the schema change in CashPilot-android PR Test: verify database schema creation and earnings CRUD #6appscolumn onworkerstable (auto-migrated viaALTER TABLEon startup)system_info.device_type == "android") have their apps converted to service entries for the main dashboard — they appear like any other deployed service with running/stopped statusCompanion PR
CashPilot-android #6 — moves apps from being faked as
containersto a proper top-levelappsfieldTest plan
appsarray stores and displays correctlyappscolumn on existing databases without data lossSummary by CodeRabbit
New Features
UI / Content