-
Notifications
You must be signed in to change notification settings - Fork 28
release: v0.9.6 #223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release: v0.9.6 #223
Conversation
WalkthroughVersion bump from 0.9.5 to 0.9.6 across project metadata, headers, and tests. Documentation updated in CHANGELOG. Manifest entries removed in .openapi-generator/FILES. Sync OAuth2 token URL now derived via a credentials parser. Minor whitespace/style changes in sync REST code and tests. Example requirements updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Sync ApiClient
participant OAuth2 as OAuth2Client
participant Cred as Credentials
participant IdP as Token Endpoint
Client->>OAuth2: request token (client credentials)
OAuth2->>Cred: parse issuer from configuration.api_issuer
Cred-->>OAuth2: token_url = _parse_issuer(api_issuer)
OAuth2->>IdP: POST token_url (User-Agent 0.9.6)
IdP-->>OAuth2: access_token / error
OAuth2-->>Client: token or raise
note over OAuth2,Cred: New: token URL derived via credentials parser
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
Hello there! How soon can this be released? Need the scope parameter in the client configuration. Not able to proceed without it. |
68d5d10
to
9e8d40f
Compare
changes: - No changes to the client interface - Minor formatting changes API commit: [c0b62b28b14d0d164d37a1f6bf19dc9d39e5769b](openfga/api@c0b62b2)
9e8d40f
to
0cdebeb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #223 +/- ##
=======================================
Coverage 70.73% 70.73%
=======================================
Files 134 134
Lines 10884 10884
=======================================
Hits 7699 7699
Misses 3185 3185 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hi @saadshaikh3 - we're aiming to release it soon, once reviewed. Should be today or tomorrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openfga_sdk/api_client.py (1)
264-270
: Fix bug: assign _retry_params.min_wait_in_ms to min_wait_in_ms (not max_retry)_min_wait_in_ms is incorrectly written into max_retry; this occurs in both async and sync clients — change the assignment in both files.
Locations:
- openfga_sdk/api_client.py — line 268
- openfga_sdk/sync/api_client.py — line 267
Apply:
if _retry_params is not None: if _retry_params.max_retry is not None: max_retry = _retry_params.max_retry if _retry_params.min_wait_in_ms is not None: - max_retry = _retry_params.min_wait_in_ms + min_wait_in_ms = _retry_params.min_wait_in_ms if _retry_params.max_wait_in_sec is not None: max_wait_in_sec = _retry_params.max_wait_in_sec
🧹 Nitpick comments (10)
openfga_sdk/oauth2.py (1)
93-94
: Make User-Agent version dynamic in both OAuth2 clientsHardcoded User-Agent contains "0.9.6" in both clients — replace with the package version to avoid editing on each release.
Files: openfga_sdk/oauth2.py (lines 93–94), openfga_sdk/sync/oauth2.py (line 93).
Within the header dict in both files:
- "User-Agent": "openfga-sdk (python) 0.9.6", + "User-Agent": f"openfga-sdk (python) {__version__}",Add near imports (outside this hunk):
from openfga_sdk import __version__openfga_sdk/configuration.py (1)
536-542
: Avoid hardcoding SDK version in debug report — use package versionReplace the hardcoded string with the package's single source of truth.
Location: openfga_sdk/configuration.py:541
return ( "Python SDK Debug Report:\n" f"OS: {sys.platform}\n" f"Python Version: {sys.version}\n" "Version of the API: 1.x\n" - "SDK Package Version: 0.9.6" + f"SDK Package Version: {__version__}" )Add near top-level imports:
from openfga_sdk import __version__pyproject.toml (1)
11-18
: Make project.version dynamic with Hatch (avoid manual version bumps)Verification: the supplied check failed (AssertionError) — pyproject.toml still contains a literal version = "0.9.6" (File: pyproject.toml, lines 11–18).
Replace the literal version with a dynamic version and Hatch config:
[project] # remove: version = "0.9.6" dynamic = ["version"] [tool.hatch.version] path = "openfga_sdk/__init__.py"After applying, re-run the validation:
python - <<'PY' import tomllib, pathlib t = tomllib.loads(pathlib.Path("pyproject.toml").read_text()) assert "version" not in t["project"] print("OK: project.version is dynamic") PYtest/sync/open_fga_api_test.py (1)
1845-1845
: Stop hardcoding the SDK UA in tests; derive it from the client constant to reduce churn.Replace the literal UA with the library's DEFAULT_USER_AGENT so version bumps don't require test edits.
Apply this diff at both call sites:
- "User-Agent": "openfga-sdk python/0.9.6", + "User-Agent": DEFAULT_USER_AGENT,Add the import once near the top of the file:
from openfga_sdk.sync.api_client import DEFAULT_USER_AGENTAlso applies to: 1898-1898
test/oauth2_test.py (1)
87-87
: Avoid hardcoded OAuth2 UA; build it from the package version.The OAuth2 UA format is stable; use
openfga_sdk.__version__
to keep tests future‑proof.- "User-Agent": "openfga-sdk (python) 0.9.6", + "User-Agent": f"openfga-sdk (python) {openfga_sdk.__version__}",Add this import once near the top:
import openfga_sdkAlso applies to: 314-314, 369-369, 424-424, 479-479, 596-596
test/api/open_fga_api_test.py (1)
1786-1786
: Use client UA constant instead of hardcoding the version.This removes test churn on every release.
- "User-Agent": "openfga-sdk python/0.9.6", + "User-Agent": DEFAULT_USER_AGENT,Add once near the top:
from openfga_sdk.api_client import DEFAULT_USER_AGENTAlso applies to: 1839-1839
test/sync/oauth2_test.py (1)
87-87
: Parameterize OAuth2 UA with the package version.Build the UA string using
openfga_sdk.__version__
to avoid per‑release edits.- "User-Agent": "openfga-sdk (python) 0.9.6", + "User-Agent": f"openfga-sdk (python) {openfga_sdk.__version__}",Add once near the top:
import openfga_sdkAlso applies to: 145-145, 205-205
CHANGELOG.md (1)
3-10
: Normalize CHANGELOG tag naming and confirm PR #220 merged
- Use v-prefixed tag in the heading and compare link for consistency — CHANGELOG.md (lines 3-10).
-### [0.9.6](https://github.com/openfga/python-sdk/compare/v0.9.5...0.9.6) (2025-09-15) +### [v0.9.6](https://github.com/openfga/python-sdk/compare/v0.9.5...v0.9.6) (2025-09-15)
- PR #220 is merged (mergedAt: 2025-09-16T04:52:29Z, state: MERGED).
openfga_sdk/sync/oauth2.py (1)
73-74
: Expose a public issuer parser and stop calling the private _parse_issuer
- Add a public API (e.g., Credentials.parse_issuer or a shared util), update callers that use the private method (openfga_sdk/oauth2.py:73, openfga_sdk/sync/oauth2.py:73 and tests in test/credentials_test.py), and ensure both sync and async flows use the same parser for parity.
test/sync/rest_test.py (1)
630-631
: Also assert PoolManager isn't used when proxy is configured.To harden the branch guard, patch urllib3.PoolManager too and assert it wasn't called.
-@patch("ssl.create_default_context") -@patch("urllib3.ProxyManager") -def test_ssl_context_used_with_proxy_manager(mock_proxy_manager, mock_create_context): +@patch("ssl.create_default_context") +@patch("urllib3.ProxyManager") +@patch("urllib3.PoolManager") +def test_ssl_context_used_with_proxy_manager( + mock_pool_manager, mock_proxy_manager, mock_create_context +): @@ - RESTClientObject(configuration=mock_config) + client = RESTClientObject(configuration=mock_config) @@ 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"} + mock_pool_manager.assert_not_called() + assert client.pool_manager is mock_proxy_manager.return_valueAlso applies to: 660-662
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (18)
.openapi-generator/FILES
(0 hunks)CHANGELOG.md
(1 hunks)VERSION.txt
(1 hunks)example/example1/requirements.txt
(1 hunks)example/example1/setup.py
(1 hunks)openfga_sdk/__init__.py
(1 hunks)openfga_sdk/api_client.py
(1 hunks)openfga_sdk/configuration.py
(1 hunks)openfga_sdk/oauth2.py
(1 hunks)openfga_sdk/sync/api_client.py
(1 hunks)openfga_sdk/sync/oauth2.py
(2 hunks)openfga_sdk/sync/rest.py
(1 hunks)pyproject.toml
(1 hunks)test/api/open_fga_api_test.py
(2 hunks)test/oauth2_test.py
(7 hunks)test/sync/oauth2_test.py
(3 hunks)test/sync/open_fga_api_test.py
(2 hunks)test/sync/rest_test.py
(8 hunks)
💤 Files with no reviewable changes (1)
- .openapi-generator/FILES
🧰 Additional context used
🧬 Code graph analysis (1)
openfga_sdk/sync/oauth2.py (1)
openfga_sdk/credentials.py (5)
_parse_issuer
(180-205)configuration
(167-171)configuration
(174-178)api_issuer
(95-99)api_issuer
(102-106)
🔇 Additional comments (13)
openfga_sdk/configuration.py (1)
541-542
: to_debug_report version updated — looks good.pyproject.toml (1)
13-13
: Project version bumped to 0.9.6 — OK.openfga_sdk/__init__.py (1)
13-13
: version updated — looks good.example/example1/setup.py (1)
18-18
: Example setup requires 0.9.6 — aligned with repo.example/example1/requirements.txt (1)
7-7
: Approve — example requirements consistent with setup.py (openfga-sdk >= 0.9.6).
Confirmed example/example1/requirements.txt and example/example1/setup.py both specify openfga-sdk >= 0.9.6. Publish the example after the release so users can pip install the new version.VERSION.txt (1)
1-1
: Version bump OK; gate merge after #220.Repo search for "0.9.5" found matches only in CHANGELOG.md (lines 5 and 10); no other occurrences outside changelog/build artifacts. Ensure PR #220 is merged, then rebase this branch and rerun the release checklist before tagging 0.9.6.
openfga_sdk/sync/oauth2.py (1)
93-95
: UA bump looks correct and consistent with tests.
No further action needed.openfga_sdk/sync/rest.py (1)
158-158
: No functional change.
Extra blank line is harmless; fine to keep or remove per formatter.openfga_sdk/sync/api_client.py (1)
41-41
: UA constant bump LGTM.
Matches test expectations and other modules.test/sync/rest_test.py (4)
538-539
: Decorator order and SSL context kwarg assertion look correct.Bottom-most patch (urllib3.PoolManager) maps to the first arg, matching the function signature. Verifying ssl_context via call kwargs is appropriate.
Also applies to: 562-562
565-566
: Client cert load path covered; kwarg check is solid.Good verification of load_cert_chain and that the created context is passed through to PoolManager.
Also applies to: 594-594
597-601
: verify_ssl=False behavior validated correctly.Checks for check_hostname=False and verify_mode=ssl.CERT_NONE are appropriate and tied to the created context used by PoolManager.
Also applies to: 627-627
665-669
: Reuse optimization test is precise.Asserting a single create_default_context call and reuse via call kwargs is sufficient; the reset-and-build step guards regressions.
Also applies to: 692-692
|
||
|
||
DEFAULT_USER_AGENT = "openfga-sdk python/0.9.5" | ||
DEFAULT_USER_AGENT = "openfga-sdk python/0.9.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
DEFAULT_USER_AGENT updated — good.
To prevent future PR churn, derive from version.
Apply:
-DEFAULT_USER_AGENT = "openfga-sdk python/0.9.6"
+DEFAULT_USER_AGENT = f"openfga-sdk python/{__version__}"
Add (outside this hunk) near imports:
from openfga_sdk import __version__
Check all UA strings reference the current version:
🏁 Script executed:
#!/bin/bash
rg -nP 'User-Agent.*0\.9\.\d+' openfga_sdk | cat
Length of output: 174
DEFAULT_USER_AGENT updated — good. Remaining hardcoded User-Agent strings found; update them to derive from version.
Apply (api_client.py):
-DEFAULT_USER_AGENT = "openfga-sdk python/0.9.6"
+DEFAULT_USER_AGENT = f"openfga-sdk python/{__version__}"
Add near imports:
from openfga_sdk import __version__
Files to fix (from verification):
- openfga_sdk/oauth2.py:93
- openfga_sdk/sync/oauth2.py:93
Replace:
"User-Agent": "openfga-sdk (python) 0.9.6",
with:
"User-Agent": f"openfga-sdk (python) {__version__}",
and add from openfga_sdk import __version__
to those modules' imports.
🤖 Prompt for AI Agents
In openfga_sdk/oauth2.py and openfga_sdk/sync/oauth2.py around line 93, replace
the hardcoded User-Agent header value with one derived from the package version:
add an import line `from openfga_sdk import __version__` to each module's
imports and change the header value from the literal "User-Agent": "openfga-sdk
(python) 0.9.6" to use f"openfga-sdk (python) {__version__}" so the User-Agent
reflects the package __version__ dynamically.
@patch("ssl.create_default_context") | ||
@patch("urllib3.PoolManager") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fragile assertion: MagicMock makes the "secure defaults not modified" check vacuous.
hasattr() on MagicMock returns True and attributes are truthy Mocks, so the assertions would pass even if code regressed. Assert concrete values instead and pre-set expected defaults.
Apply this diff to make the assertions meaningful:
- # Verify SSL verification settings were NOT modified (verify_ssl=True)
- # check_hostname and verify_mode should remain at their default secure 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
- )
+ # Verify SSL verification settings were NOT modified (verify_ssl=True)
+ assert mock_ssl_context.check_hostname is True
+ assert mock_ssl_context.verify_mode == ssl.CERT_REQUIRED
And initialize the defaults on the mocked context (outside the changed lines, right after creating mock_ssl_context):
# after: mock_ssl_context = MagicMock()
mock_ssl_context.check_hostname = True
mock_ssl_context.verify_mode = ssl.CERT_REQUIRED
Also applies to: 732-739, 744-745
🤖 Prompt for AI Agents
In test/sync/rest_test.py around lines 704-705 (and similarly for ranges 732-739
and 744-745), the test relies on hasattr() and truthy MagicMock attributes which
makes the "secure defaults not modified" check vacuous; after creating
mock_ssl_context (right after mock_ssl_context = MagicMock()) set concrete
default attributes like check_hostname = True and verify_mode =
ssl.CERT_REQUIRED on the mock, then replace hasattr()/truthy checks with
assertions that those attributes equal the concrete expected values so the test
fails if the code mutates them.
Description
This should only be merged after #220 , as it will require updating a file added in that PR
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
main
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation