Skip to content

XSUP-80841 | fix(qualys): fail fast on Qualys 1960 in test and get-assets - #46573

Open
DanielTal87 wants to merge 1 commit into
masterfrom
XSUP-80841
Open

DanielTal87 wants to merge 1 commit into
masterfrom
XSUP-80841

Conversation

@DanielTal87

Copy link
Copy Markdown
Contributor

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: https://jira-dc.paloaltonetworks.com/browse/XSUP-80841
related: https://jira-dc.paloaltonetworks.com/browse/XSUP-76520

Description

Problem

Qualys allows a limited number of simultaneous calls to the host list detection API per account (2 for the reporting account: x-concurrency-limit-limit: 2). When both slots are busy, Qualys answers 409 with Error Code 1960 ("This API cannot be run again until 1 currently running instance has finished").

Since 3.4.10 (XSUP-76520), _request_with_concurrency_retry backs off 60s and retries twice on 1960. That retry was meant for fetch-assets, but every caller of get_host_list_detection inherits it, including test-module and qualys-get-assets. Any 1960 makes them run ≥ 120s and fail, which is past the 60s instance test timeout (request timed out after 1m0s).

Findings

  • The customer runs 3.5.1. A qualys-get-assets debug log shows 3 detection requests 60s apart, all 409/1960 with x-concurrency-limit-running: 2, failing after 124.5s.
  • The slots were held by our own integration, not a deadlock. The initial assets fetch was ingesting normally (runs chained via nextTrigger, total assets 68,981 → 146,529 in ~35 min) and held one slot for minutes per run. The other was held by an earlier instance test that kept running in the background up to the 5-minute script limit after the UI gave up at 60s.
  • Reproduced in a unit test: 3 calls / 120s of back-off on current code, 1 call / 0s on the code before 3.4.10, and green with the retry disabled.
  • The fetch-assets OOM kills mentioned in the ticket happened on 3.4.13, before the 3.5.0 memory improvements; none since the customer upgraded. No change needed here.

Fix

  • get_host_list_detection gets retry_on_concurrency_limit (default True), and _request_with_concurrency_retry gets max_retries.
  • get_host_list_detections_events (used only by test-module and qualys-get-assets) passes False. On 1960 it fails immediately with a clear message ("Qualys concurrency limit reached (Error Code 1960) … usually because the assets and vulnerabilities fetch is running. Try again after it finishes."), keeping the original Qualys error.
  • The fetch-assets path (request_host_list_page) and get_vulnerabilities keep the existing 60s × 2 back-off. Fetch behavior and last_run are unchanged.
  • Added debug logs: retries allowed, host list request duration, and the x-concurrency-limit-running/limit headers on each 1960.
  • Docker image updated to demisto/python3:3.12.15.14069087. Version 3.5.1 → 3.5.2.

Tests

  • test_interactive_host_list_fails_fast_on_concurrency_limit (test-module, qualys-get-assets): 1 request, no sleep, clear message. It failed before the fix with assert 3 == 1.
  • test_host_list_skips_concurrency_retry_when_disabled, test_host_list_without_retry_returns_first_success.
  • test_interactive_host_list_keeps_other_errors_unchanged (401, 500, 409/1965): other errors pass through unchanged.
  • The existing fetch retry tests are unchanged and passing.

Must have

  • Tests
  • Documentation

@content-bot

Copy link
Copy Markdown
Contributor

🤖 AI-Powered Code Review Available

You can leverage AI-powered code review to assist with this PR!

Available Commands:

  • @marketplace-ai-reviewer start review - Initiate a full AI code review
  • @marketplace-ai-reviewer re-review - Incremental review for new commits

@DanielTal87 DanielTal87 self-assigned this Nov 11, 2026
@DanielTal87 DanielTal87 added docs-approved ready-for-pipeline-running Whether the pr is ready for running the whole pipeline, including testing on SAAS machines ready-for-ai-review The PR is ready for reviewing the PR with the AI Reviewer. labels Nov 11, 2026
@github-actions

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/qualys/Integrations/Qualysv2
   Qualysv2.py11301798%1937, 2092, 2240, 2351–2353, 2374–2376, 2387, 2602, 2606, 2610, 2612, 3205, 3261, 4136
TOTAL11301798% 

Tests Skipped Failures Errors Time
308 0 💤 0 ❌ 0 🔥 3.675s ⏱️

@marketplace-ai-reviewer marketplace-ai-reviewer removed the ready-for-ai-review The PR is ready for reviewing the PR with the AI Reviewer. label Nov 11, 2026
@marketplace-ai-reviewer

Copy link
Copy Markdown
Contributor

🤖 Analysis started. Please wait for results...

@marketplace-ai-reviewer

Copy link
Copy Markdown
Contributor
🤖 AI Review Disclaimer

This review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause.

@marketplace-ai-reviewer marketplace-ai-reviewer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi there, thanks for your contribution!

Overall the changes look good, with just a minor suggestion to improve our debug logging by including the exception traceback when hitting concurrency limits.

Once that's addressed, we should be good to go!

@dtroushinsky, @JasBeilin, @DanielTal87 please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.

except DemistoException as e:
if not Client._is_concurrency_limit_error(getattr(e, "res", None)):
raise
demisto.debug("[HTTP Error] Host list request hit the concurrency limit; failing fast for the interactive call.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider including the exception traceback in the debug log via traceback.format_exc():

Suggested change
demisto.debug("[HTTP Error] Host list request hit the concurrency limit; failing fast for the interactive call.")
demisto.debug(
f"[HTTP Error] Host list request hit the concurrency limit; failing fast for the interactive call.\n{traceback.format_exc()}"
)

@content-bot

Copy link
Copy Markdown
Contributor

Validate summary
If the AG100 validation in the pre-commit GitHub Action fails, the pull request cannot be force-merged.

Verdict: PR can be force merged from validate perspective? ✅

@content-bot

Copy link
Copy Markdown
Contributor

🔍 AI Triage Report Available

An automated triage report has been generated for this pipeline.

Status: failed
Report ID: 0144119235f4bd8f

📋 Triage Report
💡 Resolutions are available in the full report.

⚠️ AI-generated triage. Validate before acting.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-approved ready-for-pipeline-running Whether the pr is ready for running the whole pipeline, including testing on SAAS machines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants