[2.7 backport] AAP-57614 fix: dispatch save_indirect_host_entries from artifacts_handler#16365
Draft
[2.7 backport] AAP-57614 fix: dispatch save_indirect_host_entries from artifacts_handler#16365
Conversation
…dler The artifacts_handler and handle_success_and_failure_notifications can run in either order after job completion. Since event_queries_processed defaults to True on the Job model, when the notification handler runs first it sees True (the default) and skips dispatching save_indirect_host_entries. When artifacts_handler runs later and sets event_queries_processed to False, no task is dispatched to process the EventQuery records, leaving event_queries_processed stuck at False and no IndirectManagedNodeAudit records created. Fix by also dispatching save_indirect_host_entries from artifacts_handler after EventQuery records are created. The task's select_for_update lock prevents duplicate processing if both code paths dispatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit dispatched save_indirect_host_entries from artifacts_handler, but used delay_update to set event_queries_processed to False. delay_update only queues the write for the final job status save, so save_indirect_host_entries would read the default (True) from the DB and bail out before processing. Replace delay_update(event_queries_processed=False) with a direct Job.objects.filter().update() call so the value is visible in the DB before save_indirect_host_entries runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
save_indirect_host_entries calls fetch_job_event_query which reads job.installed_collections from the DB. When dispatched from artifacts_handler, installed_collections was still only in delay_update (not yet flushed to DB), so the task found no matching EventQuery records and created no IndirectManagedNodeAudit entries. Write both event_queries_processed and installed_collections directly to the DB before dispatching, so save_indirect_host_entries has all the data it needs immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Dispatching save_indirect_host_entries from artifacts_handler was fundamentally flawed: it ran before job events were written to the DB by the callback receiver, so the task found no events to process, set event_queries_processed=True, and blocked all future processing. Remove the dispatch and the now-unused import. The existing events_processed_hook (called from both the task runner after the final save and the callback receiver after the wrapup event) handles dispatching at the right time — after events are in the DB. The direct DB write of event_queries_processed=False and installed_collections (added in the previous commit) remains: it ensures events_processed_hook sees the correct values regardless of which call site runs first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
djulich
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| db_updates = {'event_queries_processed': False} | ||
| if 'installed_collections' in query_file_contents: | ||
| db_updates['installed_collections'] = query_file_contents['installed_collections'] | ||
| Job.objects.filter(id=self.instance.id).update(**db_updates) |
There was a problem hiding this comment.
Direct DB write skipped if earlier code raises exception
Medium Severity
The old code called delay_update(event_queries_processed=False) at the top of the if success: block, ensuring the flag was set even if later code raised an exception. The new direct DB write at lines 316–319 is placed at the end, after the EventQuery save loop. If an unhandled exception occurs in that loop (e.g., an IntegrityError from instance.save() that the except ValidationError doesn't catch), the direct DB write is skipped, event_queries_processed stays at the default True, and save_indirect_host_entries is never dispatched. The fallback task also won't recover this since it queries for event_queries_processed=False.
|
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.
Jira
https://issues.redhat.com/browse/AAP-57614
Summary
Forward-port of ansible/tower#7376 to
devel(AAP 2.7).save_indirect_host_entriesis never dispatched whenartifacts_handlerruns afterhandle_success_and_failure_notificationsevent_queries_processedandinstalled_collectionsdirectly to the DB instead of usingdelay_update, soevents_processed_hooksees the correct values regardless of execution orderartifacts_handler; rely on the existingevents_processed_hookwhich runs after events are in the DBTest plan
test_indirect_node_counting_external_queries.pyvia CI pipelineIndirectManagedNodeAuditrecords are created after job completionevent_queries_processedtransitions fromFalsetoTrueafter processing🤖 Generated with Claude Code
Classification
Test Results
Note
Medium Risk
Adjusts job completion/indirect host processing coordination by changing when
event_queries_processed(and related fields) are written, which could impact post-run hooks and task dispatch timing. Scope is small but touches job lifecycle behavior and DB update ordering.Overview
Fixes a race in
RunnerCallback.artifacts_handlerwhere delayed field writes could causeevents_processed_hookto miss that indirect host processing is required.When
ansible_data.jsonis present, the handler now writesevent_queries_processed=False(andinstalled_collectionswhen available) directly to theJobrow instead of relying ondelay_update, ensuring the flag is visible before wrap-up processing runs.Written by Cursor Bugbot for commit 1a205af. This will update automatically on new commits. Configure here.