Skip to content

perf(importers): add defer_product_grading option + harden watson index-flush hook#15165

Open
dogboat wants to merge 10 commits into
DefectDojo:devfrom
dogboat:misc-import-updates
Open

perf(importers): add defer_product_grading option + harden watson index-flush hook#15165
dogboat wants to merge 10 commits into
DefectDojo:devfrom
dogboat:misc-import-updates

Conversation

@dogboat

@dogboat dogboat commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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_grading importer option

New ImporterOptions flag (default False), honored by both the importer and
reimporter
. 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 is
affected. 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 the
    intermediate-flush hook into a no-op. watson.skip_index_update() only marks its
    context invalid on __exit__, so while a skip block is still accumulating, the flush
    hook 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 in
    this makes the skip actually hold.
  • Singleton guard — the hook now only intermediate-flushes the global singleton
    context. The async reindex task builds its own SearchContextManager and is itself
    the 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

  • Guards a reimporter debug log behind logger.isEnabledFor(DEBUG): it previously
    built 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 importer
    and reimporter.
  • test_watson_intermediate_flush.py — ad-hoc contexts don't drain;
    suppress_intermediate_flush() no-ops and restores.

[sc-13510]

@dogboat dogboat requested review from Maffooch and mtesauro as code owners July 7, 2026 00:47
@mtesauro mtesauro requested review from valentijnscholten and removed request for mtesauro July 8, 2026 05:52

@valentijnscholten valentijnscholten left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

1 similar comment
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Maffooch pushed a commit that referenced this pull request Jul 9, 2026
…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>
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