Skip to content

Conversation

rhamzeh
Copy link
Member

@rhamzeh rhamzeh commented Sep 12, 2025

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

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

  • New Features

    • Added support for OAuth2 scopes in credential configuration.
  • Bug Fixes

    • Sync client now reuses the SSL context to improve connection stability and performance.
  • Chores

    • Bumped SDK version to 0.9.6, updated User-Agent accordingly.
    • Updated example project dependencies to require the new SDK version.
  • Documentation

    • Updated changelog to include the 0.9.6 release notes and comparisons.

Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Walkthrough

Version 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

Cohort / File(s) Summary
Project versioning
VERSION.txt, pyproject.toml, openfga_sdk/__init__.py, CHANGELOG.md
Bump version to 0.9.6 and update CHANGELOG section headers and entries.
User-Agent/version strings
openfga_sdk/api_client.py, openfga_sdk/oauth2.py, openfga_sdk/sync/api_client.py, openfga_sdk/configuration.py
Update DEFAULT_USER_AGENT and version strings from 0.9.5 to 0.9.6; update debug report version.
Sync OAuth2 token URL derivation
openfga_sdk/sync/oauth2.py
Change token_url construction to use self._credentials._parse_issuer(configuration.api_issuer); update User-Agent to 0.9.6.
Examples min SDK version
example/example1/requirements.txt, example/example1/setup.py
Raise minimum openfga-sdk requirement from >=0.9.5 to >=0.9.6.
Generator manifest
.openapi-generator/FILES
Remove README.md and test/__init__.py from the manifest.
Sync REST whitespace
openfga_sdk/sync/rest.py
Insert one blank line after __init__ docstring; no functional change.
Tests: UA expectations
test/api/open_fga_api_test.py, test/oauth2_test.py, test/sync/open_fga_api_test.py, test/sync/oauth2_test.py
Update expected User-Agent strings to 0.9.6.
Tests: style adjustments
test/sync/rest_test.py
Quote/style normalization for patch targets and dict keys; no logic change.

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

release

Suggested reviewers

  • ttrzeng

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "release: v0.9.6" is concise and directly reflects the primary purpose of the changeset: a version/release bump to 0.9.6, as evidenced by updates to VERSION.txt, pyproject.toml, version, CHANGELOG, User-Agent strings, and related tests and examples. It is clear, specific, and appropriate for a release PR so a teammate scanning history will understand the main change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch release/v0.9.6

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

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

@saadshaikh3
Copy link

Hello there! How soon can this be released? Need the scope parameter in the client configuration. Not able to proceed without it.

changes:
- No changes to the client interface
- Minor formatting changes

API commit: [c0b62b28b14d0d164d37a1f6bf19dc9d39e5769b](openfga/api@c0b62b2)
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.73%. Comparing base (ee6c726) to head (0cdebeb).

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.
📢 Have feedback on the report? Share it here.

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

@rhamzeh rhamzeh marked this pull request as ready for review September 16, 2025 18:01
@rhamzeh rhamzeh requested a review from a team as a code owner September 16, 2025 18:01
@rhamzeh
Copy link
Member Author

rhamzeh commented Sep 16, 2025

Hi @saadshaikh3 - we're aiming to release it soon, once reviewed. Should be today or tomorrow

Copy link
Contributor

@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: 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 clients

Hardcoded 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 version

Replace 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")
PY
test/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_AGENT

Also 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_sdk

Also 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_AGENT

Also 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_sdk

Also 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_value

Also applies to: 660-662

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee6c726 and 0cdebeb.

⛔ 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"
Copy link
Contributor

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.

Comment on lines +704 to +705
@patch("ssl.create_default_context")
@patch("urllib3.PoolManager")
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

@rhamzeh rhamzeh added this pull request to the merge queue Sep 16, 2025
Merged via the queue into main with commit 5ff4a74 Sep 16, 2025
33 of 36 checks passed
@rhamzeh rhamzeh deleted the release/v0.9.6 branch September 16, 2025 19:27
@coderabbitai coderabbitai bot mentioned this pull request Oct 6, 2025
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.

4 participants