Skip to content

fix: avoid delete in loop in inventory import#16366

Merged
pb82 merged 1 commit intodevelfrom
AAP-68817
Mar 24, 2026
Merged

fix: avoid delete in loop in inventory import#16366
pb82 merged 1 commit intodevelfrom
AAP-68817

Conversation

@pb82
Copy link
Copy Markdown
Contributor

@pb82 pb82 commented Mar 24, 2026

SUMMARY

For inventory import, avoid running delete in a loop, this is very slow for when the number of hosts is large. Instead, collect all relations that have to be removed with one query and then pass all ids to delete in one go.

This will still create a number of delete queries for large imports, because the batch size is 500, but it still brings the number of queries down significantly.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
STEPS TO REPRODUCE AND EXTRA INFO
devel

Summary by CodeRabbit

  • Bug Fixes

    • Overwrite inventory import now reliably removes stale group and host memberships and performs relationship removals more efficiently.
  • Tests

    • Added a functional test verifying overwrite imports remove stale memberships while preserving the underlying inventory objects.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4403c79d-566a-40c3-a08e-08948551ef69

📥 Commits

Reviewing files that changed from the base of the PR and between bcf7d33 and 2e365f4.

📒 Files selected for processing (2)
  • awx/main/management/commands/inventory_import.py
  • awx/main/tests/functional/commands/test_inventory_import.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • awx/main/tests/functional/commands/test_inventory_import.py
  • awx/main/management/commands/inventory_import.py

📝 Walkthrough

Walkthrough

Batch-based removal of group→child and group→host memberships in the inventory import command replaces per-object iteration; adds a functional test ensuring stale memberships are removed on overwrite while underlying objects remain.

Changes

Cohort / File(s) Summary
Inventory import command
awx/main/management/commands/inventory_import.py
Refactored _delete_group_children_and_hosts to materialize filtered querysets per batch and call remove(*objs) for children and hosts, skip empty removals, update group counters by the number removed, and emit per-removed-item debug logs.
Functional test for membership removal
awx/main/tests/functional/commands/test_inventory_import.py
Added test_overwrite_removes_stale_memberships which runs two consecutive imports (with overwrite=True) via a patched loader returning different data sets, asserting stale child-group and host memberships are removed while the referenced host/group records persist.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main optimization: avoiding delete operations in loops during inventory import, which directly reflects the core change.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch AAP-68817

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.

@pb82 pb82 enabled auto-merge (squash) March 24, 2026 14:38
Signed-off-by: Peter Braun <pbraun@redhat.com>
@sonarqubecloud
Copy link
Copy Markdown

@pb82 pb82 merged commit ab29438 into devel Mar 24, 2026
19 checks passed
@pb82 pb82 deleted the AAP-68817 branch March 24, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants