Skip to content

Fix/XSUP-72726/retry mechanism in BaseClient for older urllib3 that does not support the back-off jitter#45035

Draft
MosheEichler wants to merge 2 commits into
masterfrom
fix/XSUP-72726/retry-mechanisem-in-BaseClient
Draft

Fix/XSUP-72726/retry mechanism in BaseClient for older urllib3 that does not support the back-off jitter#45035
MosheEichler wants to merge 2 commits into
masterfrom
fix/XSUP-72726/retry-mechanisem-in-BaseClient

Conversation

@MosheEichler

Copy link
Copy Markdown
Contributor

Status

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

Related Issues

https://jira-dc.paloaltonetworks.com/browse/XSUP-72726

Description

Added support for older urllib3 that does not support the back-off jitter

Must have

  • Tests
  • Documentation

@MosheEichler MosheEichler requested a review from DeanArbel July 12, 2026 13:17
@MosheEichler MosheEichler added release-notes-only Indicates that this pull request has ONLY release notes to review for documentation process 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 Jul 12, 2026
@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

@github-actions

Copy link
Copy Markdown

⚠️ CommonServerPython change detected

This PR modifies one of the runtime-injected helper scripts (CommonServerPython.py / CommonServerPowerShell.ps1 / CommonServer.js).
A change here can impact every integration and script in the repository, so the standard PR pipeline is not enough.

Required before merge:

  1. Trigger the content nightly pipeline against this branch.
  2. Make sure the nightly run is green (all jobs passed).
  3. Add the nightly-run-passed label to this PR so this check turns green.

If you believe nightly is not required for this change, please justify it in a PR comment and add the label to unblock the check.

@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 Jul 12, 2026
@marketplace-ai-reviewer

Copy link
Copy Markdown
Contributor

🤖 Analysis started. Please wait for results...

@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown

Coverage

Tests Skipped Failures Errors Time
7287 269 💤 7 ❌ 0 🔥 5m 5s ⏱️

marketplace-ai-reviewer

This comment was marked as outdated.

@MosheEichler

Copy link
Copy Markdown
Contributor Author

@tcarmeli1 tcarmeli1 added the ready-for-ai-review The PR is ready for reviewing the PR with the AI Reviewer. label Jul 12, 2026
- Retry is instantiated without the backoff_jitter kwarg (no TypeError is raised)
- The https:// adapter is mounted on the session with the configured retry object
"""
import CommonServerPython
Then:
- Retry is instantiated with the backoff_jitter kwarg set to the given value
"""
import CommonServerPython
- Retry is instantiated without the backoff_jitter kwarg (no TypeError is raised)
- The https:// adapter is mounted on the session with the configured retry object
"""
import CommonServerPython
Then:
- Retry is instantiated with the backoff_jitter kwarg set to the given value
"""
import CommonServerPython
@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 Jul 12, 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.

Thanks for your contribution! I've reviewed the PR and have a few quick updates for you to look into. Please ensure the vendor's name is added to the metadata keywords, bump the pack version, and safeguard against a potential TypeError with older urllib3 versions in the tests. Let me know if you have any questions!

Additionally, please address the following file-level notes:

  • Packs/Base/pack_metadata.json: The vendor's name must appear in the keywords field.
  • Packs/Base/Scripts/CommonServerPython/CommonServerPython.py: Reminder: Pack version bump required.

@github-code-quality[bot], @marketplace-ai-reviewer, @DeanArbel, @MosheEichler please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.


def __init__(self, *args, **kwargs):
received_kwargs.update(kwargs)
super().__init__(*args, **kwargs)

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.

Prevent potential TypeError in environments with older urllib3.

@content-bot

Copy link
Copy Markdown
Contributor

Validate summary
The following errors were thrown as a part of this pr: .
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? ✅

@julieschwartz18 julieschwartz18 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.

@MosheEichler Doc review complete, looks good! Adding the label docs-approved

@content-bot

content-bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Triage Report Available

An automated triage report has been generated for this pipeline.

Status: failed
Report ID: c8a0ce0786d6cb3e

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

⚠️ AI-generated triage. Validate before acting.

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 release-notes-only Indicates that this pull request has ONLY release notes to review for documentation process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants