chore: Fix incorrect Management API docs in Python SDK#675
Conversation
WalkthroughDocumentation rewrites the Management API usage to a standalone ManagementClient initialized with client credentials (optionally via dotenv), replaces OAuth-wrapper examples with snake_case API properties (e.g., Changes
Sequence Diagram(s)sequenceDiagram
participant FastAPI as FastAPI App
participant MgmtClient as ManagementClient
participant Auth as Auth Server
participant MgmtAPI as Management API
FastAPI->>MgmtClient: call users_api.get_users(...)
MgmtClient->>Auth: request token (client_credentials) -- if no valid token cached
Auth-->>MgmtClient: access_token (expires_at)
MgmtClient->>MgmtAPI: GET /users with access_token
MgmtAPI-->>MgmtClient: 200 OK (users)
MgmtClient-->>FastAPI: return users
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1408-1412: The snippet imports load_dotenv and will raise
ModuleNotFoundError unless python-dotenv is installed; update the docs near the
example to explicitly state the prerequisite and installation command (e.g.,
"pip install python-dotenv") and mention that load_dotenv() must be called
before initializing ManagementClient to load environment variables; reference
the import load_dotenv and the ManagementClient usage so the reader knows where
the note applies.
- Around line 1573-1580: The get_user endpoint currently catches broad Exception
and returns the raw error in HTTPException; change get_user to catch the
SDK-specific error type (e.g., ApiException or the SDK's typed error from
management.users_api.get_user_data) and handle it explicitly: map known SDK
status codes to appropriate FastAPI responses and return sanitized error
messages (no raw exception text) in HTTPException(detail=...), while logging the
full error server-side (using your logger) for diagnostics; leave a generic 500
catch-all that logs the error but returns a non-sensitive generic message.
|
Kindly resolve the coderabbit issues and it should be good. |
tamalchowdhury
left a comment
tamalchowdhury
left a comment
There was a problem hiding this comment.
Resolve the coderabbit issues and it should be ready to go.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/content/docs/developer-tools/sdks/backend/python-sdk.mdx (1)
1566-1593: The defensive check forApiException.statusis not necessary in this context.
ApiExceptionis OpenAPI-generated and always includes an HTTP status code when raised from a response. Thee.statusattribute will be populated as an integer in all normal exception flows. Adding a defensive fallback like502introduces unnecessary complexity for documentation code that should demonstrate the standard pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx` around lines 1566 - 1593, The docs example should rely on the OpenAPI-generated ApiException having a populated status, so in the get_user handler remove any defensive fallback for ApiException.status and raise HTTPException using e.status directly; update the exception block that references ApiException, keeping logger.error("Kinde API error: %s %s", e.status, e.body) and replacing any code like "status_code=e.status or 502" with "status_code=e.status" so the example shows the standard pattern for ApiException handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1597-1604: Update the documentation text describing
ManagementClient's refresh behavior: remove the hardcoded "60 seconds before
expiration" claim and instead state that the client requests new tokens via the
client credentials grant when existing tokens expire, and that token lifetime is
configurable in the Kinde portal per application; adjust the sentence
referencing "Automatic re-acquisition" to reflect this and keep the rest of the
token acquisition/caching/shared-token wording unchanged (refer to the
ManagementClient description block).
---
Nitpick comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1566-1593: The docs example should rely on the OpenAPI-generated
ApiException having a populated status, so in the get_user handler remove any
defensive fallback for ApiException.status and raise HTTPException using
e.status directly; update the exception block that references ApiException,
keeping logger.error("Kinde API error: %s %s", e.status, e.body) and replacing
any code like "status_code=e.status or 502" with "status_code=e.status" so the
example shows the standard pattern for ApiException handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6d6f0327-68c6-4c56-b385-bf0e0357ca3e
📒 Files selected for processing (1)
src/content/docs/developer-tools/sdks/backend/python-sdk.mdx
|
I'll resolve CAai comment tonight. |
|
Ok, just now looking at it and I realized it is a forked repo branch. So I skip working on this myself. @Koosha-Owji will you be addressing this any time soon? Curious to see if we can get this reviewed and merged in. Thank you |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/content/docs/developer-tools/sdks/backend/python-sdk.mdx (2)
1418-1424: Minor: brittle host parsing in the dotenv example.
os.getenv("KINDE_HOST", "").replace("https://", "")only strips thehttps://prefix and silently leaves any trailing slash orhttp://scheme intact, which then propagates into thedomainargument. For a doc snippet this is fine in the happy path, but a slightly more defensive form helps copy-paste users:♻️ Optional tweak
-management = ManagementClient( - domain=os.getenv("KINDE_HOST", "").replace("https://", ""), - client_id=os.getenv("MGMT_API_CLIENT_ID"), - client_secret=os.getenv("MGMT_API_CLIENT_SECRET") -) +_host = os.getenv("KINDE_HOST", "").rstrip("/") +management = ManagementClient( + domain=_host.removeprefix("https://").removeprefix("http://"), + client_id=os.getenv("MGMT_API_CLIENT_ID"), + client_secret=os.getenv("MGMT_API_CLIENT_SECRET"), +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx` around lines 1418 - 1424, The dotenv example currently builds the ManagementClient domain with os.getenv("KINDE_HOST", "").replace("https://", ""), which is brittle; update the snippet that constructs the domain (the value passed to ManagementClient(domain=...)) to robustly parse KINDE_HOST (e.g., use urllib.parse.urlparse to extract netloc if a scheme is present, or strip any trailing slashes and both "http://" and "https://" prefixes) so the domain passed to ManagementClient is always a clean hostname without scheme or trailing slash.
1441-1458: Add a clarifying note about different request shapes forcreate_uservsupdate_user.The code snippets are correct:
create_usernests user data under"profile", whileupdate_useraccepts flat fields like"given_name"directly. Consider adding a one-line note explaining this difference to help readers understand why the two endpoints have different structures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx` around lines 1441 - 1458, Add a one-line clarifying note explaining that create_user expects a nested payload under create_user_request["profile"] (e.g., "profile": {"given_name": ...}) while update_user takes a flat payload via update_user_request (e.g., {"given_name": ...}); reference create_user, create_user_request, profile, update_user, and update_user_request so readers understand the different request shapes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1494-1508: The snippet uses inconsistent parameter names for
organization identifier—get_organization(code=...) vs
update_organization(org_code=...) and delete_organization(org_code=...)—which
will confuse readers; update the example to use the same parameter name across
calls (prefer org_code for consistency) or add a short note calling out the
discrepancy; specifically change the get_organization call to use org_code or
add a sentence after the snippet explaining that get_organization accepts code
while the other methods expect org_code so copy-pasting must be adjusted.
---
Nitpick comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1418-1424: The dotenv example currently builds the
ManagementClient domain with os.getenv("KINDE_HOST", "").replace("https://",
""), which is brittle; update the snippet that constructs the domain (the value
passed to ManagementClient(domain=...)) to robustly parse KINDE_HOST (e.g., use
urllib.parse.urlparse to extract netloc if a scheme is present, or strip any
trailing slashes and both "http://" and "https://" prefixes) so the domain
passed to ManagementClient is always a clean hostname without scheme or trailing
slash.
- Around line 1441-1458: Add a one-line clarifying note explaining that
create_user expects a nested payload under create_user_request["profile"] (e.g.,
"profile": {"given_name": ...}) while update_user takes a flat payload via
update_user_request (e.g., {"given_name": ...}); reference create_user,
create_user_request, profile, update_user, and update_user_request so readers
understand the different request shapes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 51418105-f500-4259-b24b-9c99cec6fb8e
📒 Files selected for processing (1)
src/content/docs/developer-tools/sdks/backend/python-sdk.mdx
|
@CodeRabbit resume review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/content/docs/developer-tools/sdks/backend/python-sdk.mdx (1)
1589-1594:⚠️ Potential issue | 🟡 MinorGuard against
e.statusbeingNone.
ApiException.statusmay beNonefor non-HTTP failures (e.g., transport/connection errors before a response is received). PassingNonetoHTTPException(status_code=...)will raise a validation error and obscure the original failure. Add a sane fallback as the prior review suggested.Proposed fix
except ApiException as e: logger.error("Kinde API error: %s %s", e.status, e.body) - raise HTTPException(status_code=e.status, detail="Failed to fetch user") + raise HTTPException(status_code=e.status or 502, detail="Failed to fetch user")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx` around lines 1589 - 1594, The ApiException handler must guard against a None status before passing it to HTTPException: in the except ApiException as e block, compute a fallback status (e.g., status_code = e.status if e.status is not None else 500) and use that when raising HTTPException; keep the existing logger.error call but include the fallback-aware status in the log/message so transport/connection failures won't cause a validation error—update the except ApiException handler (the block that currently logs "Kinde API error: %s %s" and raises HTTPException) to derive and use this safe status_code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1419-1424: The domain extraction for ManagementClient is fragile:
currently os.getenv("KINDE_HOST", "").replace("https://", "") only removes
"https://" and won't handle "http://", presence of a trailing slash, or other
URL forms; update the code that builds the domain for ManagementClient (where
KINDE_HOST is read) to robustly parse the host using a URL parser (e.g., parse
with urllib.parse.urlparse) or normalize by removing any "http(s)://" prefix and
trimming trailing slashes so the ManagementClient(domain=...) always receives a
clean host string; ensure you reference KINDE_HOST, the ManagementClient
initializer, and any helper used to normalize the host.
---
Duplicate comments:
In `@src/content/docs/developer-tools/sdks/backend/python-sdk.mdx`:
- Around line 1589-1594: The ApiException handler must guard against a None
status before passing it to HTTPException: in the except ApiException as e
block, compute a fallback status (e.g., status_code = e.status if e.status is
not None else 500) and use that when raising HTTPException; keep the existing
logger.error call but include the fallback-aware status in the log/message so
transport/connection failures won't cause a validation error—update the except
ApiException handler (the block that currently logs "Kinde API error: %s %s" and
raises HTTPException) to derive and use this safe status_code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dc0c83a8-00c6-493d-a4c7-0172af1d4909
📒 Files selected for processing (1)
src/content/docs/developer-tools/sdks/backend/python-sdk.mdx
3 participants
Description (required)
The Management API section of the Python SDK docs showed
oauth.get_management()— a method that doesn't exist on any of the SDK's OAuth classes. Rewrote the section to use the correctManagementClientclass with non-deprecated API class methods, accurate method signatures, proper FastAPI handler definitions, and correct token lifecycle documentation.Summary by CodeRabbit