perf(importers): add defer_product_grading option + harden watson index-flush hook#15165
perf(importers): add defer_product_grading option + harden watson index-flush hook#15165dogboat wants to merge 11 commits into
Conversation
valentijnscholten
left a comment
valentijnscholten
left a comment
There was a problem hiding this comment.
Funny how Opus generated the exact same code/naming on your PR as my PR.
In https://github.com/DefectDojo-Inc/dojo-pro/pull/1778 I have chosen a more elegant option. That one also adds a Celery context manager to make the async index batching work in celery tasks i.e. the background importer in pro.
Can I suggest to remove the watson parts of your PR?
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…ton (#15187) The intermediate-flush hook patches SearchContextManager.add_to_context at class level, so it also fires inside update_watson_search_index_for_model's own ad-hoc context. A task processing a full WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE batch re-dispatches a clone of itself, discards those pks unindexed, and loops forever: queue length stays ~0 while the worker is pegged republishing the same batches, and nothing gets indexed. Guard the hook so only the global singleton (the request-path context managed by AsyncSearchContextMiddleware) intermediate-flushes. Ad-hoc context managers index their own batch on end(), as stock watson does. Extracted from the hardening half of #15165 for the bugfix branch. Co-authored-by: dogboat <dogboat@users.noreply.github.com>
114 commits of dev drift resolved: - default_importer/default_reimporter: dev partitioned post-processing dispatch into per-push_to_jira batches; this branch's product_grading_option=not self.defer_product_grading carried into the partitioned loop. - middleware: kept BOTH independent additions — this branch's suppress_intermediate_flush() thread-local kill-switch AND dev's new watson_search_context_for_task(); the suppression check ported into dev's instance-level add_to_context_with_flush (the old class-level singleton guard is obsolete under dev's install). - test files: kept this branch's suppress test; merged both mock import styles in test_importers_closeold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
Refreshed to current Verified: 🤖 Generated with Claude Code |
4 participants
Summary
Two small, independent improvements for large imports/reimports, plus follow-up
hardening on the async watson index-flush hook. Both are backward compatible —
opt-in flag / no change to the default path.
defer_product_gradingimporter optionNew
ImporterOptionsflag (defaultFalse), honored by both the importer andreimporter. When enabled, product-grade recalculation is skipped during the run —
per dedupe batch, at end-of-process, and after closing old findings — so the caller
runs a single
perform_product_grading()pass afterward instead.On a large import/reimport the default path recalculates the product grade once per
dedupe batch (and again at the end), which is redundant; deferring collapses that to
one pass. The default (
False) preserves existing behavior, so no current caller isaffected. The validator documents that it's all-or-nothing: with the flag set, the
caller owns the final grade.
Watson index-flush hook hardening
suppress_intermediate_flush()— a thread-local context manager that turns theintermediate-flush hook into a no-op.
watson.skip_index_update()only marks itscontext invalid on
__exit__, so while a skip block is still accumulating, the flushhook can't tell it's a skip context and would drain it — dispatching async index
tasks for objects meant to be discarded. Wrapping a
skip_index_update()block inthis makes the skip actually hold.
context. The async reindex task builds its own
SearchContextManagerand is itselfthe drain target; without the guard it would dispatch a clone of itself and loop.
Ad-hoc contexts index their own batch on
end()as before.Minor
logger.isEnabledFor(DEBUG): it previouslybuilt a
(id, hash_code)list over every original finding on each reimport,regardless of log level.
Tests
test_importers_importer.py— importer honors the flag (per-batch dispatch off +no end grade), with a control proving the default path still grades.
test_importers_closeold.py— the close-old grade is deferred, for both importerand reimporter.
test_watson_intermediate_flush.py— ad-hoc contexts don't drain;suppress_intermediate_flush()no-ops and restores.[sc-13510]