[feature] Mass command asynchronous execution pipeline#1395
Conversation
#1344 Introduced AbstractBatchCommandOperation and AbstractCommandOperation models to support batch command execution on multiple devices, following the pattern of BatchUpgradeOperation/UpgradeOperation in openwisp-firmware-upgrader. Added batch_command_operation FK to the existing Command model to link individual commands to their parent batch. Closes #1344
📝 WalkthroughWalkthroughThis PR introduces the backend foundation for batch command execution in the OpenWISP controller. It adds three new abstract model classes: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
CI Failures: Black Formatting and System ChecksHello @dee077,
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
4-16: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd workflow concurrency to prevent duplicate CI runs.
Line 4 defines triggers, but there is no
concurrencyguard. Rapid pushes to the same PR/branch can run overlapping matrices and waste runners.Suggested patch
on: push: branches: - master - "gsoc26-*" pull_request: branches: - master - "1.1" - "1.2" - "gsoc26-*" + +concurrency: + group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 4 - 16, Add a top-level concurrency stanza to the workflow (next to the existing on: triggers) to prevent duplicate CI runs: add a concurrency key with a group expression that uniquely identifies the branch/PR (e.g. using github.workflow plus github.ref or github.event.pull_request.number when available) and set cancel-in-progress: true so newer pushes cancel older runs; update the workflow around the existing on: block to include this concurrency configuration.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openwisp_controller/connection/base/models.py`:
- Around line 817-832: The launch() method sets status="in-progress" and saves
even when resolve_devices() yields no devices, which leaves batches stuck
because no CommandOperation rows will trigger calculate_and_update_status();
update launch() to detect an empty devices queryset: after resolving devices
(resolve_devices()) and before creating CommandOperation instances, if there are
zero devices set status to a terminal state (e.g., "completed" or "no-devices"),
set total_devices=0, persist those fields via save(update_fields=[...]) and
return early so no bulk_create is attempted; keep references to
CommandOperation, total_devices, and calculate_and_update_status() unchanged.
- Around line 822-830: The loop builds a potentially huge in-memory list `ops`
before calling `CommandOperation.objects.bulk_create(ops)`, causing memory
spikes; modify the code that iterates `devices.iterator()` to create and
validate each `CommandOperation` (call `full_clean()`), accumulate them in a
small buffer, and call `CommandOperation.objects.bulk_create(buffer,
batch_size=...)` whenever the buffer reaches a chosen chunk size (then clear the
buffer) and after the loop flush any remaining items; keep references to the
same symbols (`CommandOperation`, `batch_command_operation`,
`devices.iterator()`, `full_clean`, `bulk_create`) so reviewers can find and
verify the chunking change.
- Around line 823-830: The batch launch loop in launch() only creates
CommandOperation rows via CommandOperation.objects.bulk_create(ops) leaving
their command field NULL and never invoking AbstractCommand._schedule_command,
so per-device work is never enqueued; fix by, after creating each
CommandOperation (or immediately after bulk_create), ensuring each operation
gets a scheduled command—either call the scheduling logic
(AbstractCommand._schedule_command or an equivalent method) for each
CommandOperation instance or create and assign the related Command objects and
enqueue them so operations transition out of in-progress; update the code
referencing devices.iterator(), ops, batch_command_operation and
CommandOperation to create/assign commands and invoke the scheduler for every op
so the per-device work is enqueued and can complete.
- Around line 779-805: AbstractBatchCommand.clean currently only checks org
ownership for group and location but neglects to enforce the
organization-specific command allowlist and to validate command_input against
the command schema, allowing invalid or disabled device commands to be stored;
update AbstractBatchCommand.clean to perform the same command validation that
AbstractCommand.clean does: verify that self.command is allowed for
self.organization (apply the same allowlist check), validate/deserialize
self.command_input against the command's schema (raise ValidationError on schema
errors), and ensure disabled commands are rejected — reuse or call the existing
AbstractCommand validation helper/logic rather than duplicating ad-hoc checks so
errors are consistent.
- Line 835: The code uses the hardcoded reverse accessor
self.commandoperation_set in BatchCommandOperation.calculate_and_update_status
which breaks when CommandOperation is swapped; add an explicit related_name on
AbstractCommandOperation.batch_command_operation (e.g.,
related_name="batch_command_operations") and update
BatchCommandOperation.calculate_and_update_status to use that new accessor
(replace self.commandoperation_set with the chosen related_name) ensuring
compatibility with swapper.swappable_setting("connection", "CommandOperation").
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 4-16: Add a top-level concurrency stanza to the workflow (next to
the existing on: triggers) to prevent duplicate CI runs: add a concurrency key
with a group expression that uniquely identifies the branch/PR (e.g. using
github.workflow plus github.ref or github.event.pull_request.number when
available) and set cancel-in-progress: true so newer pushes cancel older runs;
update the workflow around the existing on: block to include this concurrency
configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9644ffdf-bf70-4c83-809f-afee03e5b60c
📒 Files selected for processing (3)
.github/workflows/ci.ymlopenwisp_controller/connection/base/models.pyopenwisp_controller/connection/models.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Mark user-facing strings for translation with Django i18n helpers in Django code
Avoid unnecessary blank lines inside function and method bodies
Be careful with authentication, authorization, queryset filtering, serializers, admin behavior, cache invalidation, signals, Celery tasks, and websocket updates in Django code
Preserve validation around templates, VPN/PKI material, SSH credentials, device commands, uploaded files, URLs, and subnet/IP data
Write comments and docstrings only when they explain why code is shaped a certain way, placing them before the relevant code block instead of scattering them inside it
Files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
🧠 Learnings (5)
📚 Learning: 2026-02-24T16:24:55.443Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1233
File: .github/workflows/backport.yml:22-22
Timestamp: 2026-02-24T16:24:55.443Z
Learning: In repositories within the OpenWISP organization, it is acceptable to reference reusable workflows from other OpenWISP-controlled repos using mutable refs (e.g., master) in .github/workflows. This is permissible due to the shared trust boundary within the organization. If applying this pattern, ensure the target repos are under the same organization and maintain awareness of potential breakages from upstream mutable refs; consider pinning to a tagged version for longer-term stability when appropriate.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2026-02-24T16:25:20.080Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1233
File: .github/workflows/backport.yml:35-35
Timestamp: 2026-02-24T16:25:20.080Z
Learning: In .github/workflows/backport.yml, enforce that backport-on-comment triggers only for users with author_association MEMBE R or OWNER (COLLABORATOR excluded), reflecting maintainer feedback. Update the trigger condition to check author_association and restrict to MEMBERS/OWNERS; document rationale and PR `#1233` reference in code comments.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/connection/models.pyopenwisp_controller/connection/base/models.py
🪛 zizmor (1.25.2)
.github/workflows/ci.yml
[warning] 4-14: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
8-8: LGTM!Also applies to: 14-14, 103-103
| def clean(self): | ||
| super().clean() | ||
| if ( | ||
| self.group | ||
| and self.group.organization != self.organization | ||
| ): | ||
| raise ValidationError( | ||
| { | ||
| "group": _( | ||
| "The organization of the group doesn't match " | ||
| "the organization of the batch command operation" | ||
| ) | ||
| } | ||
| ) | ||
| if ( | ||
| self.location | ||
| and self.location.organization != self.organization | ||
| ): | ||
| raise ValidationError( | ||
| { | ||
| "location": _( | ||
| "The organization of the location doesn't match " | ||
| "the organization of the batch command operation" | ||
| ) | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
Preserve command validation on the batch model.
AbstractBatchCommand.clean() only checks org ownership for group and location. It never enforces the organization-specific command allowlist or validates command_input against the command schema, so invalid/disabled device commands can be stored here even though AbstractCommand.clean() would reject the same payload later.
As per coding guidelines, "Preserve validation around templates, VPN/PKI material, SSH credentials, device commands, uploaded files, URLs, and subnet/IP data".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openwisp_controller/connection/base/models.py` around lines 779 - 805,
AbstractBatchCommand.clean currently only checks org ownership for group and
location but neglects to enforce the organization-specific command allowlist and
to validate command_input against the command schema, allowing invalid or
disabled device commands to be stored; update AbstractBatchCommand.clean to
perform the same command validation that AbstractCommand.clean does: verify that
self.command is allowed for self.organization (apply the same allowlist check),
validate/deserialize self.command_input against the command's schema (raise
ValidationError on schema errors), and ensure disabled commands are rejected —
reuse or call the existing AbstractCommand validation helper/logic rather than
duplicating ad-hoc checks so errors are consistent.
Source: Coding guidelines
| def launch(self): | ||
| self.status = "in-progress" | ||
| self.save() | ||
| devices = self.resolve_devices() | ||
| CommandOperation = load_model("connection", "CommandOperation") | ||
| ops = [] | ||
| for device in devices.iterator(): | ||
| op = CommandOperation( | ||
| batch_command_operation=self, | ||
| device=device, | ||
| ) | ||
| op.full_clean() | ||
| ops.append(op) | ||
| CommandOperation.objects.bulk_create(ops) | ||
| self.total_devices = len(ops) | ||
| self.save(update_fields=["total_devices"]) |
There was a problem hiding this comment.
Empty launches get stuck in in-progress.
If resolve_devices() returns no rows, this method still persists status="in-progress" and then saves total_devices=0. Because no CommandOperation rows exist, nothing will ever call calculate_and_update_status(), so the batch has no path out of that state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openwisp_controller/connection/base/models.py` around lines 817 - 832, The
launch() method sets status="in-progress" and saves even when resolve_devices()
yields no devices, which leaves batches stuck because no CommandOperation rows
will trigger calculate_and_update_status(); update launch() to detect an empty
devices queryset: after resolving devices (resolve_devices()) and before
creating CommandOperation instances, if there are zero devices set status to a
terminal state (e.g., "completed" or "no-devices"), set total_devices=0, persist
those fields via save(update_fields=[...]) and return early so no bulk_create is
attempted; keep references to CommandOperation, total_devices, and
calculate_and_update_status() unchanged.
| ops = [] | ||
| for device in devices.iterator(): | ||
| op = CommandOperation( | ||
| batch_command_operation=self, | ||
| device=device, | ||
| ) | ||
| op.full_clean() | ||
| ops.append(op) | ||
| CommandOperation.objects.bulk_create(ops) |
There was a problem hiding this comment.
Chunk the operation creation path for large batches.
ops grows to one Python object per target device before the single bulk_create(). For a mass-command feature, that turns launch into an avoidable memory spike and one oversized insert. Flush in chunks and pass batch_size instead of materializing the full batch in memory first.
As per coding guidelines, "Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openwisp_controller/connection/base/models.py` around lines 822 - 830, The
loop builds a potentially huge in-memory list `ops` before calling
`CommandOperation.objects.bulk_create(ops)`, causing memory spikes; modify the
code that iterates `devices.iterator()` to create and validate each
`CommandOperation` (call `full_clean()`), accumulate them in a small buffer, and
call `CommandOperation.objects.bulk_create(buffer, batch_size=...)` whenever the
buffer reaches a chosen chunk size (then clear the buffer) and after the loop
flush any remaining items; keep references to the same symbols
(`CommandOperation`, `batch_command_operation`, `devices.iterator()`,
`full_clean`, `bulk_create`) so reviewers can find and verify the chunking
change.
Source: Coding guidelines
| for device in devices.iterator(): | ||
| op = CommandOperation( | ||
| batch_command_operation=self, | ||
| device=device, | ||
| ) | ||
| op.full_clean() | ||
| ops.append(op) | ||
| CommandOperation.objects.bulk_create(ops) |
There was a problem hiding this comment.
launch() never enqueues any per-device command work.
This loop only inserts CommandOperation rows. command stays NULL, AbstractCommand._schedule_command() is never reached, and nothing in the provided models transitions the child operations away from in-progress. As written, a launched batch cannot actually execute commands or complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openwisp_controller/connection/base/models.py` around lines 823 - 830, The
batch launch loop in launch() only creates CommandOperation rows via
CommandOperation.objects.bulk_create(ops) leaving their command field NULL and
never invoking AbstractCommand._schedule_command, so per-device work is never
enqueued; fix by, after creating each CommandOperation (or immediately after
bulk_create), ensuring each operation gets a scheduled command—either call the
scheduling logic (AbstractCommand._schedule_command or an equivalent method) for
each CommandOperation instance or create and assign the related Command objects
and enqueue them so operations transition out of in-progress; update the code
referencing devices.iterator(), ops, batch_command_operation and
CommandOperation to create/assign commands and invoke the scheduler for every op
so the per-device work is enqueued and can complete.
| self.save(update_fields=["total_devices"]) | ||
|
|
||
| def calculate_and_update_status(self): | ||
| operations = self.commandoperation_set |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Django, what determines the default reverse accessor name for a ForeignKey, and does swapping/customizing the related model class change the generated _set accessor?
💡 Result:
In Django, the default reverse accessor name for a ForeignKey is determined by taking the lowercased name of the model that defines the field and appending _set [1][2]. Specifically, the logic (found in django/db/models/fields/reverse_related.py) constructs this name as {model_name}_set, unless a custom related_name is provided [2]. If you swap or customize the related model class—for example, by using a swappable model like settings.AUTH_USER_MODEL—the generated reverse accessor on the related model depends on the model name of the target class itself [2]. Because the reverse accessor is defined on the related (target) model, if the swapped model has a different class name (e.g., swapping a generic User model for a custom CustomUser), the resulting {model_name}_set accessor will be updated to reflect the name of the new, active model class. Consequently, code relying on the hardcoded _set suffix may break if the swapped model class name differs from the original. To avoid these naming issues or potential clashes, it is recommended to explicitly set a related_name on the ForeignKey [3][4]. This decouples the reverse accessor from the specific model name and prevents automatic name generation that could change if the related model is swapped [3].
Citations:
- 1: https://docs.djangoproject.com/en/5.2/ref/models/options/
- 2: https://github.com/django/django/blob/6d4e5feb79f7eabe8a0c7c4b87f25b1a7f87ca0b/django/db/models/fields/reverse_related.py
- 3: https://django.readthedocs.io/en/6.0.x/ref/models/fields.html
- 4: https://stackoverflow.com/questions/72162495/django-reverse-accessor-clashed-with-reverse-accessor
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant pieces in the repo
rg -n "batch_command_operation" -S .
rg -n "commandoperation_set" -S openwisp_controller/connection/base/models.py
# Show around the line referenced by the review comment (835)
sed -n '780,880p' openwisp_controller/connection/base/models.pyRepository: openwisp/openwisp-controller
Length of output: 4266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "batch_command_operation" -S .
rg -n "commandoperation_set" -S openwisp_controller/connection/base/models.py
sed -n '780,880p' openwisp_controller/connection/base/models.pyRepository: openwisp/openwisp-controller
Length of output: 4266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '430,520p' openwisp_controller/connection/base/models.py
sed -n '860,960p' openwisp_controller/connection/base/models.py
rg -n "class AbstractCommandOperation|class CommandOperation|swappable|related_name" openwisp_controller/connection/base/models.pyRepository: openwisp/openwisp-controller
Length of output: 5777
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '430,520p' openwisp_controller/connection/base/models.py
sed -n '860,960p' openwisp_controller/connection/base/models.py
rg -n "class AbstractCommandOperation|class CommandOperation|swappable|related_name" openwisp_controller/connection/base/models.pyRepository: openwisp/openwisp-controller
Length of output: 5777
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "swappable|CONNECTION_.*COMMAND_.*OPERATION|CommandOperation" openwisp_controller/connection/base/models.py
sed -n '880,980p' openwisp_controller/connection/base/models.pyRepository: openwisp/openwisp-controller
Length of output: 2258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "swappable|CONNECTION_.*COMMAND_.*OPERATION|CommandOperation" openwisp_controller/connection/base/models.py
sed -n '880,980p' openwisp_controller/connection/base/models.pyRepository: openwisp/openwisp-controller
Length of output: 2258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "class CommandOperation" -S .
rg -n "CONNECTION_.*COMMAND.*OPERATION|COMMAND_OPERATION|COMMAND_OPERATION_MODEL" -S .
rg -n "swappable" -S openwisp_controller
rg -n "batch_command_operation\s*=\s*models\.ForeignKey" -S openwisp_controllerRepository: openwisp/openwisp-controller
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,200p' openwisp_controller/connection/models.py
rg -n "swappable|get_model_name\\(\"connection\"|CONNECTION_.*COMMAND.*OPERATION|COMMAND_.*OPERATION" -S openwisp_controller/connection/models.py openwisp_controller/connection/*.py openwisp_controller/**/*.pyRepository: openwisp/openwisp-controller
Length of output: 5568
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,200p' openwisp_controller/connection/models.py
rg -n "swappable|CONNECTION_.*COMMAND.*OPERATION|COMMAND_.*OPERATION|get_model_name\\(\"connection\"\\s*,\\s*\"CommandOperation\"\\)" -S openwisp_controllerRepository: openwisp/openwisp-controller
Length of output: 6547
Don’t hardcode the reverse accessor on a swappable relation
self.commandoperation_set comes from the default reverse accessor of AbstractCommandOperation.batch_command_operation, but the FK has no related_name, and CommandOperation is swappable (swapper.swappable_setting("connection", "CommandOperation")). With a custom swapped CommandOperation model, the default <model>_set accessor can change, breaking the aggregation. Add an explicit related_name on AbstractCommandOperation.batch_command_operation and use that accessor in BatchCommandOperation.calculate_and_update_status().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openwisp_controller/connection/base/models.py` at line 835, The code uses the
hardcoded reverse accessor self.commandoperation_set in
BatchCommandOperation.calculate_and_update_status which breaks when
CommandOperation is swapped; add an explicit related_name on
AbstractCommandOperation.batch_command_operation (e.g.,
related_name="batch_command_operations") and update
BatchCommandOperation.calculate_and_update_status to use that new accessor
(replace self.commandoperation_set with the chosen related_name) ensuring
compatibility with swapper.swappable_setting("connection", "CommandOperation").
2 participants
Checklist
Reference to Existing Issue
Closes #1344.
Description of Changes
BatchCommand+CommandOperationmodels: parent-child batch execution pipeline mirroring the firmware upgrader'sBatchUpgradeOperation/UpgradeOperationpattern, with organization scoping, group/location targeting, and status aggregation from child operationsToDo: