Skip to content

Ariadne Codegen path, and VCRpy fixtures#116

Merged
ejfine merged 28 commits intomainfrom
ariadgne
Dec 11, 2025
Merged

Ariadne Codegen path, and VCRpy fixtures#116
ejfine merged 28 commits intomainfrom
ariadgne

Conversation

@ejfine
Copy link
Contributor

@ejfine ejfine commented Dec 11, 2025

Why is this change necessary?

Needed to make standard VCRpy configuration fixtures more easily accessible to child templates

Update pre-commit exclude paths based on consolidated ariadne codegen for GraphQL

How does this change address the issue?

creates the VCRpy fixtures

What side effects does this change have?

N/A

How is this change tested?

Downstream repos

Other

Started updating some of the python package versions to include the version specified

Summary by CodeRabbit

  • New Features

    • VCRpy integration for Python templates to record/playback network interactions in tests.
    • Optional artifact download step for Pulumi workflows.
  • Chores

    • Updated CI action versions, pre-commit hooks, and various tool/version pins (UV, pytest, Node toolchain, etc.).
    • Updated DevContainer extensions and code-analysis exclusion patterns.
    • Shell script tightened to fail on unset variables and pipe errors.

✏️ Tip: You can customize this high-level summary in your review settings.

@ejfine ejfine self-assigned this Dec 11, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Warning

Rate limit exceeded

@ejfine has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 32 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 90f9ebd and 085bdba.

📒 Files selected for processing (1)
  • template/copier_template_resources/{% if template_might_want_to_use_vcrpy %}vcrpy_fixtures.py{% endif %} (1 hunks)

Walkthrough

Updates dependency and tooling versions across devcontainers, CI, pre-commit and context metadata; adds optional VCRpy testing support (new copier config, template question, fixtures), introduces Pulumi workflow artifact inputs, tightens shell error handling in a helper script, and adjusts path exclusions for type-checking/linting.

Changes

Cohort / File(s) Summary
Devcontainer versions & context
/.devcontainer/devcontainer.json, template/.devcontainer/devcontainer.json.jinja-base
Bumped coderabbit.coderabbit-vscode extension 0.16.0 → 0.16.1 and updated devcontainer context hash.
Installer tooling
/.devcontainer/install-ci-tooling.py
Bumped UV_VERSION from 0.9.11 → 0.9.17 (no other logic changes).
GitHub Actions workflows
/.github/workflows/ci.yaml, /.github/workflows/tag-on-merge.yaml
Updated action versions: actions/checkout v5.0.0 → v6.0.1, actions/setup-python v6.0.0 → v6.1.0, actions/cache v4.2.4 → v4.3.0; preserved step structure/inputs.
Pre-commit & typecheck excludes
/.pre-commit-config.yaml, /pyrightconfig.json
Bumped many pre-commit hook revisions; changed exclude pattern **/graphql_codegen**/generated/graphql; added **/copier_template_resources to pyright exclusions.
Project dependency
/pyproject.toml
Bumped pytest pin from >=9.0.1>=9.0.2.
Context/version metadata
/extensions/context.py, template/extensions/context.py.jinja-base
Updated many version literals (pins and minimum ranges), added new keys (e.g., graphql_tools_mock_version, vcrpy_version, pytest_recording_version, pytest_asyncio_version) and adjusted image/runtime defaults. No control-flow changes.
Copier template config
/copier.yaml, template/copier.yml.jinja-base
Added boolean template_might_want_to_use_vcrpy (default: no, conditioned on Python) and new configure_vcrpy question guarded by that flag.
VCRpy pytest fixtures
template/copier_template_resources/{% if template_might_want_to_use_vcrpy %}vcrpy_fixtures.py{% endif %}
Added vcrpy_fixtures.py defining ALLOWED_HOSTS, CUSTOM_ALLOWED_HOSTS, vcr_config() and pytest_recording_configure() to set allowed hosts, match rules, and request/response sanitization hooks for cassette recording.
Template Pulumi workflow
template/template/.github/workflows/{% if template_uses_pulumi %}pulumi-aws.yml{% endif %}.jinja-base
Added workflow inputs DOWNLOAD_ARTIFACT_NAME and DOWNLOAD_ARTIFACT_PATH, plus a conditional step to download artifacts when a name is provided.
Test template/data updates
template/tests/copier_data/data1.yaml.jinja-base, template/tests/copier_data/data2.yaml.jinja-base, tests/copier_data/data1.yaml, tests/copier_data/data2.yaml
Added template_might_want_to_use_vcrpy flag in test data and inserted configure_vcrpy conditional blocks in template test data.
Shell strictness change
/.devcontainer/windows-host-helper.sh
Replaced set -e with set -euo pipefail to treat unset vars as errors and enable pipefail behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review focus suggestions:
    • template/copier_template_resources/...vcrpy_fixtures.py: verify pytest-recording hooks, header sanitization, and type/casting robustness.
    • /.devcontainer/windows-host-helper.sh: ensure callers do not rely on unset variables and CI Windows flows still behave.
    • extensions/context.py and template variant: validate version changes and min-version vs exact-pin intent.
    • CI/workflow updates and .pre-commit-config.yaml/pyrightconfig.json: check compatibility with hosted runners and exclude patterns.

Possibly related PRs

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title partially relates to the changeset—it mentions 'VCRpy fixtures' and 'Ariadne Codegen path', which are addressed in the PR. However, the title omits the broader scope of dependency version updates across multiple files (GitHub Actions, pre-commit hooks, Python packages) which constitute a significant portion of the changes.
Description check ✅ Passed The PR description addresses most required template sections (Why, How, Side effects, Testing, Other) but the 'Link to Issue or Message thread' section is completely missing, and some explanations lack detail. The description covers the main objectives but could be more comprehensive.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
extensions/context.py (1)

14-77: Version constants look fine; consider reducing duplication/drift risk

From a code perspective, the new/updated context keys (e.g. pytest_version, pydantic_version, vcrpy_version, pytest_recording_version, pytest_asyncio_version, graphql_tools_mock_version, front‑end versions, and GHA action versions) are straightforward assignments and will flow cleanly into templates.

The main concern here is maintenance and drift:

  • Several comments already flag that these values are duplicated across install-ci-tooling.py, pyproject.toml, CI workflows, and tests data files.
  • Each future bump will require touching multiple places, which is error‑prone.

If feasible, it would be good to:

  • Centralize these versions (e.g., a single versions manifest consumed by this hook, CI, and pyproject), or
  • At minimum, document a “version bump checklist” and keep the comments tightly in sync with the actual duplication points so future maintainers don’t miss any locations.

Functionally this is fine as‑is; this is a maintainability/process suggestion rather than a blocker.

Please verify that these new version strings match the versions used in pyproject.toml, CI workflows, and any tests/data*.yml or copier.yaml entries so the template surface stays consistent.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da332bc and 5a5d0bd.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .devcontainer/devcontainer.json (2 hunks)
  • .devcontainer/install-ci-tooling.py (1 hunks)
  • .github/workflows/ci.yaml (4 hunks)
  • .github/workflows/tag-on-merge.yaml (1 hunks)
  • .pre-commit-config.yaml (6 hunks)
  • copier.yaml (1 hunks)
  • extensions/context.py (6 hunks)
  • pyproject.toml (1 hunks)
  • pyrightconfig.json (1 hunks)
  • template/.devcontainer/devcontainer.json.jinja-base (1 hunks)
  • template/copier.yml.jinja-base (1 hunks)
  • template/copier_template_resources/{% if template_might_want_to_use_vcrpy %}vcrpy_fixtures.py{% endif %} (1 hunks)
  • template/extensions/context.py.jinja-base (2 hunks)
  • template/template/.github/workflows/{% if template_uses_pulumi %}pulumi-aws.yml{% endif %}.jinja-base (2 hunks)
  • template/tests/copier_data/data1.yaml.jinja-base (1 hunks)
  • template/tests/copier_data/data2.yaml.jinja-base (1 hunks)
  • tests/copier_data/data1.yaml (1 hunks)
  • tests/copier_data/data2.yaml (1 hunks)
🔇 Additional comments (19)
pyproject.toml (1)

7-13: Pytest bump looks straightforward; confirm plugin/tooling compatibility

Raising the minimum to pytest>=9.0.2 is a reasonable incremental bump and aligns with the broader testing/tooling updates in the PR. Please double‑check that:

  • All pytest plugins used in downstream templates support 9.0.2+
  • CI images and any pinned environments are using at least this version
.devcontainer/install-ci-tooling.py (1)

8-8: UV version bump is isolated; verify availability across runners

Updating UV_VERSION to 0.9.17 keeps the installer logic intact. Please confirm:

  • 0.9.17 is available at the astral.sh install URLs used here (Windows install.ps1 and POSIX install.sh)
  • This version matches what you expect in CI and any docs/examples
template/.devcontainer/devcontainer.json.jinja-base (1)

27-41: Devcontainer template extension bump is consistent; keep in sync with base devcontainer

Bumping coderabbit.coderabbit-vscode to 0.16.1 in the template looks good and matches the non‑templated devcontainer. Just make sure this version exists/publishes cleanly in the VS Code marketplace so template consumers don’t hit a resolution error.

.devcontainer/devcontainer.json (1)

21-41: Devcontainer updates align with template and hashing hook

The VS Code extension bump to coderabbit.coderabbit-vscode@0.16.1 plus the updated devcontainer context hash are consistent with the templated file and the pre‑commit hash hook. No behavioral changes beyond the extension version are introduced.

Also applies to: 65-65

.pre-commit-config.yaml (3)

44-56: Pre-commit hook version bumps look consistent; validate SHAs and behavior

You’ve updated several hooks to newer frozen SHAs (typos, mirrors-prettier, check-jsonschema). Config structure and arguments are unchanged, so behavior should be equivalent modulo upstream fixes/features. Please verify:

  • Each rev corresponds to the tagged version noted in the comment (e.g., v3.7.4, 0.35.0)
  • No new defaults/behavioral changes in these hook versions conflict with existing expectations (especially formatting/validation in large repos)

Also applies to: 110-137, 197-204


251-280: Ruff hook bump and generated/graphql exclusions align with Ariadne path change

The ruff-pre-commit rev bump plus the new exclude patterns:

  • ./*/generated/graphql/.* for src/tests and format hooks

match the described shift from graphql_codegen to generated/graphql. This should keep generated GraphQL code out of lint/format runs as intended. Confirm that:

  • All generated GraphQL artifacts now live under generated/graphql (and not under the old path)
  • There isn’t any hand-written code under generated/graphql that you still want linted

281-307: Pylint and pyright pre-commit excludes updated to new generated path

The updated excludes for:

  • Pylint (.*/generated/graphql/.*)
  • Local pyright hook (.*/generated/graphql/.*)

keep them in sync with the ruff configuration and the new Ariadne output directory. This is consistent with treating that folder as generated code. Just verify that template files and other non-generated sources aren’t unintentionally pulled under generated/graphql in downstream templates.

pyrightconfig.json (1)

3-18: Pyright excludes align with new generated and template resource paths

Adding:

  • "**/generated/graphql" to mirror the Ariadne codegen path change
  • "**/venv" alongside "**/.venv"
  • "**/copier_template_resources" to ignore template‑only helper modules

keeps type checking focused on real source while avoiding generated code and copier template artifacts. This looks coherent with the pre‑commit configuration. Confirm that there’s no non‑generated, runtime Python you expect Pyright to check under these directories.

.github/workflows/ci.yaml (2)

28-36: Pre-commit job action bumps are minimal; watch for upstream behavior changes

Updating actions/checkout, actions/setup-python, and actions/cache to newer major/minor versions in the pre-commit job is a straightforward maintenance step. Since GitHub Actions occasionally tweak defaults (e.g., fetch depth, caching behavior), please confirm:

  • No new required inputs or breaking changes were introduced for these versions
  • Cache keys behave as expected across runs with v4.3.0

Also applies to: 52-59


86-94: Lint-matrix job keeps actions in sync with pre-commit job

Mirroring the same action version bumps in lint-matrix keeps CI consistent across jobs, which is good for predictability. As with the other job, just validate that:

  • The newer actions/setup-python still installs the requested patch versions (3.12.7, 3.13.1) without surprises
  • The actions/cache@v4.3.0 interaction with your mutex step and .precommit_cache directory remains stable

Also applies to: 136-144

.github/workflows/tag-on-merge.yaml (1)

17-21: Checkout action bump in tag-on-merge workflow is low risk

Upgrading to actions/checkout@v6.0.1 while keeping the ref, fetch-depth, and persist-credentials settings unchanged should preserve behavior. Please confirm that:

  • The action still honors github.event.pull_request.merge_commit_sha as expected
  • There are no new permission or default-branch nuances introduced with v6
template/template/.github/workflows/{% if template_uses_pulumi %}pulumi-aws.yml{% endif %}.jinja-base (1)

79-88: LGTM! Artifact download inputs and step are properly configured.

The new workflow inputs and conditional download step are well-structured. The artifact download only executes when DOWNLOAD_ARTIFACT_NAME is non-empty, which provides flexibility without breaking existing workflows that don't need artifacts.

Also applies to: 120-125

tests/copier_data/data1.yaml (1)

14-14: LGTM! Test data correctly includes the new VCRpy flag.

tests/copier_data/data2.yaml (1)

16-16: LGTM! Test data correctly enables VCRpy for this test case.

Good practice to have test data covering both enabled and disabled states.

template/tests/copier_data/data1.yaml.jinja-base (1)

10-10: LGTM! Template conditional is correctly implemented.

The variable name is correct and the conditional logic properly adds the VCRpy configuration option when enabled.

template/copier.yml.jinja-base (1)

45-50: LGTM! VCRpy configuration option is properly guarded and documented.

The double conditional (template_uses_python and template_might_want_to_use_vcrpy) ensures the VCRpy configuration option only appears in appropriate contexts. The default of "no" requires explicit opt-in, which is a safe choice.

copier.yaml (1)

69-73: LGTM! Parent VCRpy option is properly configured.

The new template_might_want_to_use_vcrpy option follows the established naming pattern, has appropriate conditional guards, and provides clear documentation. The default of "no" is a sensible choice.

template/extensions/context.py.jinja-base (1)

46-48: LGTM! Version context keys properly exposed to child templates.

The new version keys for VCRpy, pytest-recording, pytest-asyncio, and graphql-tools-mock follow the established pattern and enable child templates to reference these dependency versions. All four variables are properly defined in the parent extensions/context.py (vcrpy_version, pytest_recording_version, pytest_asyncio_version, and graphql_tools_mock_version).

template/tests/copier_data/data2.yaml.jinja-base (1)

10-10: Fix typo in template variable name.

The template variable is named template_might_to_use_vcrpy but should be template_might_want_to_use_vcrpy (missing "want"). This typo means the conditional block will never execute even when VCRpy support is enabled, breaking the intended functionality.

Apply this diff to fix the typo:

-{% endraw %}{% if template_might_to_use_vcrpy %}{% raw %}configure_vcrpy: false{% endraw %}{% endif %}{% raw %}
+{% endraw %}{% if template_might_want_to_use_vcrpy %}{% raw %}configure_vcrpy: false{% endraw %}{% endif %}{% raw %}

Likely an incorrect or invalid review comment.

Comment on lines +9 to +64
ALLOWED_HOSTS = ["testserver"] # Skip recording any requests to our own server - let them run live

CUSTOM_ALLOWED_HOSTS: tuple[str, ...] = ()

ALLOWED_HOSTS.extend(CUSTOM_ALLOWED_HOSTS)
if (
os.name == "nt"
): # on Windows (in CI), the network calls happen at a lower level socket connection even to our FastAPI test client, and can get automatically blocked. This disables that automatic network guard, which isn't great...but since it's still in place on Linux, any actual problems would hopefully get caught before pushing to CI.
ALLOWED_HOSTS.extend(["127.0.0.1", "localhost", "::1"])


@pytest.fixture(autouse=True)
def vcr_config() -> dict[str, list[str]]:
return {"allowed_hosts": ALLOWED_HOSTS}


def pytest_recording_configure(
vcr: VCR,
config: pytest.Config, # noqa: ARG001 # the config argument MUST be present (even when unused) or pytest-recording throws an error
):
vcr.match_on = cast(tuple[str, ...], vcr.match_on) # pyright: ignore[reportUnknownMemberType] # I know vcr.match_on is unknown, that's why I'm casting and isinstance-ing it...not sure if there's a different approach pyright prefers
assert isinstance(vcr.match_on, tuple), (
f"vcr.match_on is not a tuple, it is a {type(vcr.match_on)} with value {vcr.match_on}"
)
vcr.match_on += ("body",) # body is not included by default, but it seems relevant

def before_record_request(request: Request) -> Request | None:
request_headers_to_filter = ("User-Agent",)
for header in request_headers_to_filter:
if header in request.headers:
del request.headers[header]

return request

vcr.before_record_request = before_record_request

def before_record_response(response: dict[str, JsonValue]) -> dict[str, JsonValue]:
headers_to_filter = (
"Transfer-Encoding",
"Date",
"Server",
) # none of these headers in the response matter for unit testing, so might as well make the cassette files smaller
headers = response["headers"]
assert isinstance(headers, dict), (
f"Expected response['headers'] to be a dict, got {type(headers)} with value {headers}"
)
for header in headers_to_filter:
if header in headers:
del headers[header]
if (
header.lower() in headers
): # some headers are lowercased by the server in the response (e.g. Date, Server)
del headers[header.lower()]
return response

vcr.before_record_response = before_record_response
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

vcrpy default header filtering security best practices

💡 Result:

Short summary and recommended best practices for header filtering with vcrpy

Key facts

  • vcrpy records headers only if you include headers in request matching (match_on includes "headers"); default match_on does not include headers. [1]
  • vcrpy provides filter_headers to remove/replace header values when writing cassettes. Use it to strip Authorization, API keys, cookies, etc. [1][2]
  • Use before_record_request / before_record_response hooks for custom/redaction logic not covered by filter_* options. [3]

Recommended security practices

  1. Always filter sensitive headers (Authorization, Cookie, Set-Cookie, X-Api-Key, etc.) via filter_headers. Example: with my_vcr.use_cassette('c.yml', filter_headers=['authorization','cookie']). [1][2]
  2. Prefer replacing values (or dropping headers) rather than leaving originals in cassettes. Use consistent placeholders so tests remain reproducible. [2]
  3. Avoid matching on headers unless necessary. If you filter a header but still match on headers/queries, replays may fail — keep match_on and filters consistent. [1][4]
  4. Also filter query params and POST fields (filter_query_parameters, filter_post_data_parameters) for secrets in URLs or bodies. [1][2][5]
  5. Use before_record_request / before_record_response to handle nonstandard secrets or to perform complex redaction/normalization. [3]
  6. Keep real secrets out of source control: load tokens from environment variables in tests and use filtered placeholders in cassettes. Rotate/revoke tokens if accidentally committed. [2]
  7. Review cassettes before committing; consider git pre-commit checks to detect secrets. (General hygiene; see vcrpy filtering features.) [1][2]

Useful references

  • vcrpy advanced/configuration (filter_headers, filter_query_parameters, match_on). [1]
  • vcrpy examples and filter usage on PyPI/doc pages. [2][6]
  • Use before_record_request / before_record_response for custom filtering. [3]

References
[1] vcrpy documentation — Advanced / Configuration (filter_headers, match_on).
[2] vcrpy examples and filtering guidance (PyPI / docs).
[3] vcrpy API/config: before_record_request & before_record_response hooks.
[4] R vcr / protecting secrets notes on header redaction and matching caveats.
[5] filter_query_parameters & filter_post_data_parameters docs.


🌐 Web query:

pytest-recording vcr.match_on type tuple list

💡 Result:

You can use either a list or a tuple for VCR.match_on — vcrpy’s default is a tuple (match_on=('method',...)) but examples and configuration accept a list (e.g. match_on=['uri','method']). pytest-recording passes your vcr_config/marker args through to VCR, so either form works. [1][2]

Sources:

  • vcrpy configuration / API (shows default tuple and examples using list). [1]
  • pytest-recording docs (passes config through to VCR). [2]

🌐 Web query:

vcrpy before_record_request Set-Cookie Authorization header filtering

💡 Result:

Short answer + examples.

  • To remove/filter Authorization on requests: use VCR(filter_headers=...) or a before_record_request to mutate request.headers. VCR supports tuples where value can be None, static string, or callable. [1][2]

Example (remove or redact Authorization):

import vcr

my_vcr = vcr.VCR(filter_headers=[('authorization', None)])
# or
def redact_auth(key, value, request):
    return 'Bearer [REDACTED]'
my_vcr = vcr.VCR(filter_headers=[('authorization', redact_auth)])
  • To drop Set-Cookie (a response header) you should use before_record_response (not before_record_request) to mutate the response.headers or remove Set-Cookie entries before the cassette is written. [2]

Example (remove Set-Cookie):

import vcr

def drop_set_cookie(response):
    response['headers'].pop('set-cookie', None)
    return response

my_vcr = vcr.VCR(before_record_response=drop_set_cookie)
  • If you want both in one place: combine filter_headers for Authorization and before_record_response for Set-Cookie:

    my_vcr = vcr.VCR(
    filter_headers=[('authorization', None)],
    before_record_response=lambda resp: (resp['headers'].pop('set-cookie', None), resp)[1]
    )

References:

  • vcrpy API/configuration (VCR options: filter_headers, before_record_request/response). [1]
  • Advanced filtering and examples (filter_headers semantics; before_record_response examples). [2]
  • Data-filtering examples and advanced patterns. [3]

If you want, I can paste a runnable pytest example showing requests + cassette.


🏁 Script executed:

# Check if there are any vcrpy or pytest-recording version constraints in the repo
find . -type f \( -name "pyproject.toml" -o -name "requirements*.txt" -o -name "setup.py" \) -exec grep -l "vcrpy\|pytest-recording" {} \;

Repository: LabAutomationAndScreening/copier-base-template

Length of output: 72


🏁 Script executed:

# Check if there are any examples of vcrpy usage in the codebase
find . -type f -name "*.py" | xargs grep -l "vcr\|VCR" 2>/dev/null | head -20

Repository: LabAutomationAndScreening/copier-base-template

Length of output: 113


🏁 Script executed:

# Look for any existing cassette files or test examples to understand intended usage
find . -type f \( -name "*.yaml" -o -name "*.yml" \) | grep -i cassette | head -10

Repository: LabAutomationAndScreening/copier-base-template

Length of output: 72


🌐 Web query:

vcrpy filter_headers vs before_record_request header removal idiomatic approach

💡 Result:

Short answer: prefer filter_headers for simple, declarative redaction/replacement of specific header names (idiomatic for static secrets). Use before_record_request when you need conditional logic, to drop whole requests, or to mutate other parts of the request (URI, body, etc.). [1][2][3]

Why / when

  • filter_headers: built into VCR config; accepts names, name→replacement pairs, or callables for per-header replacement — concise and intended for hiding sensitive header values in cassettes. Use it when you only need to remove or replace header values for recordings. [1][3]
  • before_record_request: a callback that receives the request and may return a modified request or None to skip recording. Use it for conditional rules (e.g., only scrub headers for certain paths/hosts), complex transformations, or when you must change URI/body/match keys or drop recordings entirely. [2]

Minimal examples

  • filter_headers (idiomatic for static redaction):
    my_vcr = vcr.VCR(filter_headers=[('authorization','REDACTED'), 'cookie'])
    with my_vcr.use_cassette('c.yml'): ...
    [1][3]

  • before_record_request (conditional/complex):
    def cb(request):
    if request.path == '/login': return None # don't record
    request.headers.pop('X-Secret', None) # mutate conditionally
    return request
    my_vcr = vcr.VCR(before_record_request=cb)
    [2]

Recommendation: default to filter_headers for most header-redaction needs (simpler, explicit, readable). Reach for before_record_request only when you need per-request logic, to skip recordings, or to mutate non-header parts. [1][2][3]


🌐 Web query:

pytest-recording vcr_config fixture filter_headers example

💡 Result:

Short example — use the vcr_config fixture to pass vcrpy's filter_headers (either header-names or (name, replacement) tuples or callables):

conftest.py

import pytest

@pytest.fixture(scope="session")
def vcr_config():
    return {
        # simple: remove the header (same as ('authorization', None))
        "filter_headers": ["authorization", "x-api-key"],

        # replace header values in cassettes with a fixed token
        # (works because vcrpy accepts (name, value) tuples)
        # the value can also be a callable that returns a replacement or None
        "filter_headers": [
            ("authorization", "REDACTED"),
            ("x-api-key", lambda value: "API_KEY_REDACTED"),
        ],

        # other common options:
        "record_mode": "once",
        "ignore_localhost": True,
    }

Notes:

  • pytest-recording forwards vcr_config directly into vcrpy.use_cassette, so vcrpy's filter_headers semantics apply. [1][2]

Sources:

  • pytest-recording (vcr_config usage examples). [1]
  • vcrpy advanced features (filter_headers supports (key, value) tuples and callables). [2]

Use vcrpy's filter_headers for sensitive header redaction and convert match_on to a tuple for robustness

Good integration of pytest-recording and VCR. Two improvements:

  1. Sensitive header filtering (security):
    The current before_record_request removes only "User-Agent". For a shared template, this risks accidentally persisting credentials or PII into cassettes (e.g., Authorization, Cookie, Set-Cookie, X-API-Key).

    Instead of manually deleting headers in before_record_request and before_record_response, use vcrpy's built-in filter_headers parameter in your vcr_config fixture. This is more maintainable and idiomatic:

    @pytest.fixture(autouse=True)
    def vcr_config() -> dict[str, list[str]]:
  •  return {"allowed_hosts": ALLOWED_HOSTS}
    
  •  return {
    
  •      "allowed_hosts": ALLOWED_HOSTS,
    
  •      "filter_headers": [
    
  •          "authorization",
    
  •          "proxy-authorization",
    
  •          "cookie",
    
  •          "set-cookie",
    
  •          "x-api-key",
    
  •      ],
    
  •  }
    
    
    Then simplify or remove the manual header deletion logic in your `before_record_request` and `before_record_response` callbacks.
    
    
  1. vcr.match_on type handling (robustness):
    Lines 29–33 cast and assert that vcr.match_on is a tuple. This is brittle: vcrpy accepts both list and tuple, so if the library ever returns a list, the assertion will fail even though a simple conversion works. Replace with:

  • vcr.match_on = cast(tuple[str, ...], vcr.match_on)
  • assert isinstance(vcr.match_on, tuple), (
  •    f"vcr.match_on is not a tuple, it is a {type(vcr.match_on)} with value {vcr.match_on}"
    
  • )
  • vcr.match_on += ("body",)
  • Normalize to tuple for type-checkers and safe extension

  • vcr.match_on = tuple(vcr.match_on) + ("body",)

<!-- fingerprinting:phantom:poseidon:olive -->

<!-- This is an auto-generated comment by CodeRabbit -->

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.devcontainer/windows-host-helper.sh (1)

10-21: set -u breaks graceful argument validation.

Adding set -u causes the script to exit with an unset variable error if no arguments are passed, before the explicit check on line 18 can display the helpful "Usage" message.

When $1 is accessed on line 18 without being set, bash will immediately exit with an error like $1: unbound variable, bypassing the intended error handling.

Apply one of these fixes:

Option 1: Use safe parameter expansion to allow graceful checking:

-set -euo pipefail  # Exit immediately on error
+set -euo pipefail  # Exit immediately on error
 
 if [ -z "$BASH_VERSION" ]; then
   echo "Error: This script must be run with bash (e.g., 'bash windows-host-helper.sh')." >&2
   exit 1
 fi
 
 # Check for the git URL argument
-if [ -z "$1" ]; then
+if [ -z "${1:-}" ]; then
     echo "Usage: $0 <git-url>"
     exit 1
 fi
 
-gitUrl="$1"
+gitUrl="${1:-}"

Option 2: Check argument count explicitly:

 # Check for the git URL argument
-if [ -z "$1" ]; then
+if [ $# -eq 0 ]; then
     echo "Usage: $0 <git-url>"
     exit 1
 fi
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a5d0bd and 90f9ebd.

📒 Files selected for processing (2)
  • .devcontainer/devcontainer.json (2 hunks)
  • .devcontainer/windows-host-helper.sh (1 hunks)
🔇 Additional comments (2)
.devcontainer/windows-host-helper.sh (1)

10-10: set -o pipefail addition is good.

The pipefail option ensures pipeline failures are caught. Existing error handling (e.g., line 30's || true) is compatible with this change.

.devcontainer/devcontainer.json (1)

65-65: Context hash update is appropriately managed by pre-commit hook.

The devcontainer context hash has been updated and is protected from manual edits via the pre-commit hook as documented in the comment.

// basic tooling
// "eamodio.gitlens@15.5.1",
"coderabbit.coderabbit-vscode@0.16.0",
"coderabbit.coderabbit-vscode@0.16.1",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

CodeRabbit VS Code extension version 0.16.1 release

💡 Result:

I couldn’t find any public release or release notes for CodeRabbit VS Code extension version 0.16.1. The latest published extension version I found is 0.16.0 (updated November 19, 2025). [1][2]

Sources:

  • VsixHub listing for CodeRabbit — latest version 0.16.0 (updated Nov 19, 2025). [1]
  • CodeRabbit docs / changelog and VS Code integration announcement. [2]

Revert CodeRabbit extension to version 0.16.0 — version 0.16.1 does not exist.

The specified version 0.16.1 is not published in the VS Code marketplace; the latest available version is 0.16.0 (released November 19, 2025). This will cause devcontainer builds to fail when attempting to install the extension. Change line 25 to use 0.16.0 or wait for 0.16.1 to be officially released.

🤖 Prompt for AI Agents
.devcontainer/devcontainer.json around line 25: the extensions list pins
"coderabbit.coderabbit-vscode@0.16.1", but that version does not exist and will
break devcontainer builds; change the pinned version to the published one
"0.16.0" (or remove the explicit version if you prefer latest) so the extension
can be installed successfully during devcontainer creation.

@ejfine ejfine merged commit a71846a into main Dec 11, 2025
6 checks passed
@ejfine ejfine deleted the ariadgne branch December 11, 2025 17:55
@coderabbitai coderabbitai bot mentioned this pull request Jan 23, 2026
@coderabbitai coderabbitai bot mentioned this pull request Mar 5, 2026
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