Skip to content

SSL Handling - #111

Merged
mshriver merged 1 commit into
ibutsu:mainfrom
mshriver:ssl-handling
Sep 7, 2026
Merged

mshriver merged 1 commit into
ibutsu:mainfrom
mshriver:ssl-handling

Conversation

@mshriver

@mshriver mshriver commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

avoid race conditions for ssl context with boto and other libraries that may call truststore.inject_into_ssl

Summary by Sourcery

Ensure S3 uploads remain compatible with dynamically injected SSL trust stores while tightening type checking, diagnostics, and regression coverage.

New Features:

  • Synchronize botocore's cached SSL context with the active Python SSL context to support truststore injection and avoid SSL recursion issues.

Bug Fixes:

  • Prevent eager boto3 and botocore imports during pytest plugin loading by deferring S3 uploader imports until S3 mode is used.
  • Improve exception logging and fallback behavior across serialization and S3 operations.

Enhancements:

  • Modernize typing, datetime handling, lint configuration, and static type-checking support for boto3 and botocore.

Build:

  • Update Ruff and mypy pre-commit versions and include boto3 in mypy dependencies.

Tests:

  • Add coverage for botocore SSL context synchronization, truststore integration, lazy imports, and expanded error logging behavior.

@sourcery-ai

sourcery-ai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

The PR avoids SSL context races caused by truststore injection by lazily importing the S3 uploader and synchronizing botocore’s cached SSLContext with the active stdlib context at module load and uploader construction, with tests covering import behavior and synchronization paths.

Sequence diagram for synchronized S3 SSL setup

sequenceDiagram
    participant Pytest
    participant S3UploaderModule
    participant Sync as sync_botocore_ssl_context
    participant Truststore
    participant Botocore
    participant S3Uploader

    Truststore->>ssl: inject_into_ssl()
    Pytest->>S3UploaderModule: import s3_uploader
    S3UploaderModule->>Sync: sync_botocore_ssl_context()
    Sync->>Botocore: read httpsession.SSLContext
    Sync->>ssl: read SSLContext
    alt contexts differ
        Sync->>Botocore: assign httpsession.SSLContext = ssl.SSLContext
    end
    Pytest->>S3Uploader: __init__()
    S3Uploader->>Sync: sync_botocore_ssl_context()
    S3Uploader->>Botocore: create S3 client
    Botocore-->>S3Uploader: client uses active SSLContext
Loading

File-Level Changes

Change Details Files
Synchronize botocore with the currently active SSL context while preserving lazy boto imports.
  • Added a guarded helper that replaces botocore’s cached SSLContext when it differs from ssl.SSLContext.
  • Run synchronization when the S3 uploader module loads and again during uploader initialization.
  • Moved the S3 uploader import into the S3-only session-finish branch so normal plugin imports do not load boto3 or botocore.
src/pytest_ibutsu/s3_uploader.py
src/pytest_ibutsu/pytest_plugin.py
Added coverage for lazy imports and SSL context synchronization behavior.
  • Verify importing the plugin does not eagerly import boto3 or botocore.
  • Test context replacement, no-op behavior, exception handling, and uploader initialization synchronization.
tests/test_plugin.py
tests/test_s3_uploader.py
Updated formatting and static-analysis tooling versions and applied resulting formatting changes.
  • Bumped Ruff and mypy pre-commit revisions.
  • Reformatted lambda and test expression layout.
.pre-commit-config.yaml
tests/test_hooks.py
tests/test_pydantic_v2_validation.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot 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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.54%. Comparing base (4671122) to head (e9f406e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
+ Coverage   68.15%   69.54%   +1.38%     
==========================================
  Files           7        7              
  Lines         829      834       +5     
  Branches      140      141       +1     
==========================================
+ Hits          565      580      +15     
+ Misses        235      225      -10     
  Partials       29       29              
Flag Coverage Δ
unittests 69.54% <100.00%> (+1.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/pytest_ibutsu/archiver.py 75.00% <ø> (+1.53%) ⬆️
src/pytest_ibutsu/modeling.py 62.35% <100.00%> (ø)
src/pytest_ibutsu/pytest_plugin.py 67.81% <100.00%> (+0.10%) ⬆️
src/pytest_ibutsu/s3_uploader.py 98.46% <100.00%> (+14.53%) ⬆️
src/pytest_ibutsu/sender.py 72.80% <ø> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4671122...e9f406e. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mshriver
mshriver force-pushed the ssl-handling branch 2 times, most recently from fa2fe28 to b09dde9 Compare September 7, 2026 14:49
- Defer boto3/botocore imports until S3 upload mode is active to keep pytest plugin initialization lean.
- Add `sync_botocore_ssl_context()` to align `botocore.httpsession.SSLContext` with active `ssl.SSLContext`, preventing Python 3.12+ `RecursionError` when `truststore` is injected.
- Update mypy overrides and pre-commit configurations for boto dependencies.
- Add unit tests covering lazy import behavior, SSL context synchronization, and urllib3 context creation.
truststore handling

Co-authored-by: Claude <noreply@anthropic.com>
@mshriver
mshriver merged commit 010421f into ibutsu:main Sep 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant