release(python): v0.9.6 #614
Conversation
WalkthroughUpdates CI to point Python SDK tests at a new OpenAPI ref. Adjusts Python client release metadata (CHANGELOG and packageVersion). Adds comprehensive SSL context tests for the Python sync REST client using mocks, covering CA certs, client certs, verification toggles, proxy usage, reuse, and combined options. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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. Comment |
rhamzeh
force-pushed
the
release/python-v0.9.6
branch
2 times, most recently
from
67504cb to
14b04b6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
config/clients/python/CHANGELOG.md.mustache (1)
5-9: Avoid hardcoding the release date inside the template.The date “(2025-09-15)” will be baked into every generated changelog for {{packageVersion}}. Prefer injecting the date at release time or using a template variable, or at minimum confirm the date matches the actual release cut.
If you keep a manual date, please confirm the intended release date; today is 2025-09-16.
config/clients/python/template/test/sync/rest_test.py.mustache (3)
528-585: Patch the symbol in the module under test to avoid import-style drift.Patch targets like
"ssl.create_default_context"and"urllib3.PoolManager"work only if the SUT imports those names exactly that way. Prefer patching via the module under test so changes likefrom ssl import create_default_contextwon’t break tests.Apply this diff to the affected tests:
-@patch("ssl.create_default_context") -@patch("urllib3.PoolManager") +@patch("{{packageName}}.sync.rest.ssl.create_default_context") +@patch("{{packageName}}.sync.rest.urllib3.PoolManager") def test_ssl_context_created_with_ca_cert(...Repeat similarly for other tests:
- PoolManager → "{{packageName}}.sync.rest.urllib3.PoolManager"
- ProxyManager → "{{packageName}}.sync.rest.urllib3.ProxyManager"
- create_default_context → "{{packageName}}.sync.rest.ssl.create_default_context"
--- `620-653`: **Make ProxyManager call assertions robust across urllib3 versions (positional vs kwarg).** Some urllib3 versions accept `proxy_url` positionally; asserting only kwargs can flake. Extract from args-or-kwargs. ```diff - call_kwargs = mock_proxy_manager.call_args[1] - assert call_kwargs["ssl_context"] == mock_ssl_context - assert call_kwargs["proxy_url"] == "http://proxy:8080" - assert call_kwargs["proxy_headers"] == {"Proxy-Auth": "token"} + args, kwargs = mock_proxy_manager.call_args + proxy_url = kwargs.get("proxy_url", args[0] if args else None) + assert proxy_url == "http://proxy:8080" + assert kwargs["ssl_context"] == mock_ssl_context + assert kwargs.get("proxy_headers") == {"Proxy-Auth": "token"}
694-736: Tighten checks for secure defaults when verify_ssl=True.The “not hasattr(...) or ...” pattern can mask issues. Since you mock the context, set explicit safe defaults and assert exact values.
- assert ( - not hasattr(mock_ssl_context, "check_hostname") - or mock_ssl_context.check_hostname - ) - assert ( - not hasattr(mock_ssl_context, "verify_mode") - or mock_ssl_context.verify_mode != ssl.CERT_NONE - ) + # Set explicit defaults on the mock to make assertions strict + mock_ssl_context.check_hostname = True + mock_ssl_context.verify_mode = ssl.CERT_REQUIRED + assert mock_ssl_context.check_hostname is True + assert mock_ssl_context.verify_mode == ssl.CERT_REQUIRED
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/main.yaml(1 hunks)config/clients/python/CHANGELOG.md.mustache(1 hunks)config/clients/python/config.overrides.json(1 hunks)config/clients/python/template/test/sync/rest_test.py.mustache(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
config/clients/*/config.overrides.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
config/clients/*/config.overrides.json: Always update the packageVersion in each language-specific config.overrides.json when making version changes
Maintain FOSSA compliance notice IDs in each language’s config.overrides.json
Files:
config/clients/python/config.overrides.json
config/**/*.{json,mustache}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never hardcode API keys or credentials in configuration or template files
Files:
config/clients/python/config.overrides.jsonconfig/clients/python/CHANGELOG.md.mustacheconfig/clients/python/template/test/sync/rest_test.py.mustache
config/{common/config.base.json,clients/*/config.overrides.json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure consistent versioning across base and language override configuration files to avoid version conflicts
Files:
config/clients/python/config.overrides.json
config/**/*.mustache
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Validate mustache syntax and variable references across all template files, including CHANGELOG.md.mustache
Files:
config/clients/python/CHANGELOG.md.mustacheconfig/clients/python/template/test/sync/rest_test.py.mustache
🧠 Learnings (4)
📓 Common learnings
Learnt from: wadells
PR: openfga/sdk-generator#607
File: config/clients/python/template/src/sync/rest.py.mustache:147-158
Timestamp: 2025-09-08T22:31:40.547Z
Learning: The OpenFGA SDK generator Python templates have both sync (urllib3) and async (aiohttp) REST clients that should maintain consistent SSL context handling. Both create the SSL context once during client initialization using the same pattern: ssl.create_default_context(cafile=configuration.ssl_ca_cert) with identical cert_file and verify_ssl handling.
Learnt from: wadells
PR: openfga/sdk-generator#607
File: config/clients/python/template/src/sync/rest.py.mustache:147-158
Timestamp: 2025-09-08T22:31:40.547Z
Learning: For the OpenFGA SDK generator Python client templates, the user wadells prefers to keep the sync and async client SSL context implementations consistent rather than adding additional features to one without the other.
Learnt from: wadells
PR: openfga/sdk-generator#607
File: config/clients/python/template/src/sync/rest.py.mustache:147-158
Timestamp: 2025-09-08T22:31:40.547Z
Learning: For the OpenFGA SDK generator Python client templates, the user wadells prefers to keep the sync and async client SSL context implementations consistent rather than adding additional features to one without the other.
Learnt from: wadells
PR: openfga/sdk-generator#607
File: config/clients/python/template/src/sync/rest.py.mustache:147-158
Timestamp: 2025-09-08T22:31:40.547Z
Learning: For the OpenFGA SDK generator Python client templates, the user wadells prefers to keep the sync and async client SSL context implementations consistent rather than adding additional features to one without the other.
Learnt from: wadells
PR: openfga/sdk-generator#607
File: config/clients/python/template/src/sync/rest.py.mustache:147-158
Timestamp: 2025-09-08T22:31:40.547Z
Learning: For the OpenFGA SDK generator Python client templates, the user wadells prefers to keep the sync and async client SSL context implementations consistent rather than adding additional features to one without the other.
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
PR: openfga/sdk-generator#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Run make test-all-clients before major changes
Applied to files:
.github/workflows/main.yaml
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
PR: openfga/sdk-generator#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to config/clients/*/config.overrides.json : Always update the packageVersion in each language-specific config.overrides.json when making version changes
Applied to files:
config/clients/python/config.overrides.json
📚 Learning: 2025-09-08T22:31:40.547Z
Learnt from: wadells
PR: openfga/sdk-generator#607
File: config/clients/python/template/src/sync/rest.py.mustache:147-158
Timestamp: 2025-09-08T22:31:40.547Z
Learning: The OpenFGA SDK generator Python templates have both sync (urllib3) and async (aiohttp) REST clients that should maintain consistent SSL context handling. Both create the SSL context once during client initialization using the same pattern: ssl.create_default_context(cafile=configuration.ssl_ca_cert) with identical cert_file and verify_ssl handling.
Applied to files:
config/clients/python/template/test/sync/rest_test.py.mustache
🔇 Additional comments (4)
config/clients/python/CHANGELOG.md.mustache (1)
5-5: Double-check Unreleased formatting for the upcoming tag.Placing a version header under “Unreleased” is fine if your tooling expects it, but ensure it won’t duplicate sections when you cut v{{packageVersion}}.
config/clients/python/template/test/sync/rest_test.py.mustache (1)
655-692: Great addition: verifies SSL context reuse to address OpenSSL 3.x perf.This guards against regressions from re-creating contexts on each request. Nicely scoped and readable.
Consider adding a sister test for the async client to maintain parity with the sync implementation, per prior guidance.
.github/workflows/main.yaml (1)
136-136: OPEN_API_REF SHA verified — run codegen & Python tests.c0b62b28b14d0d164d37a1f6bf19dc9d39e5769b exists in repo openfga/api; confirm it references the intended OpenAPI spec and that codegen + the Python tests pass (CI/local).
Location: .github/workflows/main.yaml:136config/clients/python/config.overrides.json (1)
5-5: Approve — packageVersion bumped to 0.9.6; verify base/other overrides & FOSSA noticeScript shows: config/clients/python/config.overrides.json → 0.9.6; config/common/config.base.json not found; no other config/clients//config.overrides.json detected. Action: ensure config/common/config.base.json (if used) and all config/clients//config.overrides.json match 0.9.6 and that the FOSSA notice ID remains unchanged.
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
No changes to public APIs.