feat: More dry approach to API handling #149
Open
+4,329
−7,130
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughManagementClient now dynamically discovers and instantiates API classes from the management api module, injects Bearer tokens into API calls via a call_api wrapper using ManagementTokenManager, exposes snake_case API namespaces, and adds deprecated convenience wrappers delegating to those namespaces. Base URL handling was simplified. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant MC as ManagementClient
participant API as Dynamic Api Instance
participant Wrapper as call_api Wrapper
participant TokenMgr as ManagementTokenManager
participant AC as ApiClient
App->>MC: call users_api.get_users(...)
MC->>API: delegate to users Api instance
API->>Wrapper: invoke call_api(...)
Wrapper->>TokenMgr: request access token
TokenMgr-->>Wrapper: return Bearer token
Wrapper->>AC: call_api(...) with Authorization header
AC->>AC: perform HTTP request
AC-->>Wrapper: return HTTP response
Wrapper-->>API: return response to API method
API-->>MC: return result
MC-->>App: return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @kinde_sdk/management/management_client.py:
- Around line 100-123: The token-injection wrapper in _setup_token_handling must
handle failures from token_manager.get_access_token() and avoid silently
overwriting existing Authorization headers: update call_api_with_token to
try/except around self.token_manager.get_access_token(), raise or return a clear
AuthenticationError (or raise ValueError) if the call raises or returns a falsy
token instead of injecting "Bearer None", and before setting
kwargs['header_params']['Authorization'] check for an existing Authorization
value and either preserve it or log/raise a warning so you don't overwrite it
silently; ensure the error you raise is descriptive (e.g., "Failed to acquire
access token") and replace api_client.call_api only after this robust wrapper is
implemented.
- Line 281: The parameter type for get_organization should explicitly allow
None; update the signature of get_organization to use an explicit Optional type
(e.g., Optional[str] or Python 3.10+ union syntax str | None) instead of relying
on a default None, and add the corresponding import (from typing import
Optional) if using Optional to satisfy PEP 484.
- Around line 185-495: Add explicit Optional[type] annotations for parameters
that default to None: import typing.Optional and the corresponding request
models (e.g., CreateUserRequest, CreateOrganizationRequest, CreateRoleRequest,
CreateFeatureFlagRequest, UpdateUserRequest, UpdateRoleRequest,
UpdateFeatureFlagRequest) and update signatures such as
create_user(create_user_request: Optional[CreateUserRequest] = None, **kwargs),
create_organization(create_organization_request:
Optional[CreateOrganizationRequest] = None, **kwargs),
create_role(create_role_request: Optional[CreateRoleRequest] = None, **kwargs),
create_feature_flag(create_feature_flag_request:
Optional[CreateFeatureFlagRequest] = None, **kwargs), update_user(id: str,
update_user_request: Optional[UpdateUserRequest] = None, **kwargs),
update_role(role_id: str, update_role_request: Optional[UpdateRoleRequest] =
None, **kwargs), and update_feature_flag(feature_flag_key: str,
update_feature_flag_request: Optional[UpdateFeatureFlagRequest] = None,
**kwargs); keep the existing **kwargs forwarding behavior unchanged.
🧹 Nitpick comments (1)
kinde_sdk/management/management_client.py (1)
154-178: Consider handling consecutive uppercase letters in snake_case conversion.The current regex
r'(?<!^)(?=[A-Z])'will convert consecutive uppercase letters individually, which may create non-intuitive attribute names for acronym-heavy classes:
APIsApi→a_p_i_s(might preferapis)MFAApi→m_f_a(might prefermfa)This works correctly but could be refined if more natural names are preferred.
♻️ Optional enhancement for acronym handling
@staticmethod def _class_name_to_snake_case(class_name: str) -> str: """ Convert a class name to snake_case attribute name. Examples: - UsersApi -> users - FeatureFlagsApi -> feature_flags - APIsApi -> apis - MFAApi -> mfa Args: class_name: The class name (e.g., 'UsersApi') Returns: Snake case attribute name (e.g., 'users') """ # Remove the 'Api' suffix name = class_name[:-3] if class_name.endswith('Api') else class_name - # Insert underscores before uppercase letters (except at the start) - # and convert to lowercase - snake_case = re.sub(r'(?<!^)(?=[A-Z])', '_', name).lower() + # Handle sequences of uppercase letters followed by lowercase + # (e.g., "HTTPSConnection" -> "HTTPS_Connection") + s1 = re.sub('(.)([A-Z][a-z]+)', r'\1_\2', name) + # Handle transitions from lowercase/digit to uppercase + # (e.g., "getHTTPResponseCode" -> "get_HTTPResponseCode") + snake_case = re.sub('([a-z0-9])([A-Z])', r'\1_\2', s1).lower() return snake_caseThis would produce:
APIsApi→apisMFAApi→mfaHTTPSConnectionApi→https_connection
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kinde_sdk/management/management_client.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T04:46:50.368Z
Learnt from: KomanRudden
Repo: kinde-oss/kinde-python-sdk PR: 101
File: kinde_sdk/auth/permissions.py:6-7
Timestamp: 2025-08-21T04:46:50.368Z
Learning: The kinde_sdk has both frontend and management API modules. The frontend permissions_api.py exists at kinde_sdk/frontend/api/permissions_api.py and contains PermissionsApi class. Using the management API instead of the frontend API for user permissions would be incorrect.
Applied to files:
kinde_sdk/management/management_client.py
🪛 GitHub Actions: CI
kinde_sdk/management/management_client.py
[error] 231-231: pydantic.ValidationError: create_user received unexpected keyword arguments (email, given_name, family_name) when calling UsersApi.create_user.
🪛 Ruff (0.14.10)
kinde_sdk/management/management_client.py
281-281: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🔇 Additional comments (1)
kinde_sdk/management/management_client.py (1)
19-20: Theapimodule atkinde_sdk/management/apidoes not exist, causing aModuleNotFoundErrorwhenmanagement_client.pyis imported.Lines 19-20 attempt to import
from kinde_sdk.management import api, but neither theapidirectory nor anapi.pyfile exists in the repository. Additionally, the__init__.pyfile imports classes from this non-existent module (e.g.,from kinde_sdk.management.api.users_api import UsersApi), so the management module cannot be imported at all. Either the api module needs to be created with the expected structure, or the dynamic loading approach needs to be revised to work with the actual module layout.⛔ Skipped due to learnings
Learnt from: KomanRudden Repo: kinde-oss/kinde-python-sdk PR: 101 File: kinde_sdk/auth/permissions.py:6-7 Timestamp: 2025-08-21T04:46:50.368Z Learning: The kinde_sdk has both frontend and management API modules. The frontend permissions_api.py exists at kinde_sdk/frontend/api/permissions_api.py and contains PermissionsApi class. Using the management API instead of the frontend API for user permissions would be incorrect.
Comment on lines
100
to
+123
| def _setup_token_handling(self): | ||
| """Set up automatic token refresh for API calls.""" | ||
| # Token will be added directly in the API method since we're calling REST client directly | ||
| pass | ||
|
|
||
| def _generate_methods(self): | ||
| """Generate dynamic methods for each API endpoint.""" | ||
| for resource, endpoints in self.API_ENDPOINTS.items(): | ||
| # Handle special cases for singularization | ||
| if resource == 'business': | ||
| resource_singular = 'business' # Don't remove 's' from 'business' | ||
| else: | ||
| resource_singular = resource[:-1] if resource.endswith('s') else resource | ||
|
|
||
| for action, endpoint in endpoints.items(): | ||
| if len(endpoint) == 3: | ||
| method, path, query_keys = endpoint | ||
| else: | ||
| method, path = endpoint | ||
| query_keys = () | ||
|
|
||
| # Create method name based on action and resource | ||
| if action == 'list': | ||
| method_name = f"get_{resource}" | ||
| elif action == 'get': | ||
| method_name = f"get_{resource_singular}" | ||
| else: | ||
| method_name = f"{action}_{resource_singular}" | ||
|
|
||
| # Create the method | ||
| api_method = self._create_api_method(method, path, resource, action, query_keys) | ||
|
|
||
| # Set the method on the class | ||
| setattr(self, method_name, api_method) | ||
|
|
||
| def _create_api_method(self, http_method: str, path: str, resource: str, action: str, query_keys=()) -> Callable: | ||
| """ | ||
| Create a dynamic method for an API endpoint. | ||
| Set up automatic token injection for all API calls. | ||
| Args: | ||
| http_method: HTTP method (GET, POST, etc.) | ||
| path: API endpoint path | ||
| resource: API resource name (users, organizations, etc.) | ||
| action: API action (list, get, create, etc.) | ||
| Returns: | ||
| A callable method that makes the API request | ||
| This modifies the API client to automatically add the Bearer token | ||
| to all outgoing requests. | ||
| """ | ||
| # Handle special cases for singularization | ||
| if resource == 'business': | ||
| resource_singular = 'business' # Don't remove 's' from 'business' | ||
| else: | ||
| resource_singular = resource[:-1] if resource.endswith('s') else resource | ||
|
|
||
| def api_method(*args, **kwargs) -> Dict[str, Any]: | ||
| # Format path with any path parameters from args | ||
| formatted_path = path | ||
| if '{' in path and args: | ||
| param_values = list(args) | ||
| while '{' in formatted_path and param_values: | ||
| start_idx = formatted_path.find('{') | ||
| end_idx = formatted_path.find('}') | ||
| if start_idx >= 0 and end_idx >= 0: | ||
| formatted_path = formatted_path[:start_idx] + str(param_values.pop(0)) + formatted_path[end_idx + 1:] | ||
|
|
||
| # Handle query/body split | ||
| if http_method in ('GET', 'DELETE'): | ||
| query_params = {k: v for k, v in kwargs.items() if v is not None} | ||
| payload = None | ||
| else: | ||
| # Lift ONLY declared query_keys into the query string | ||
| qset = set(query_keys or ()) | ||
| query_params = {k: kwargs.pop(k) for k in list(kwargs) if k in qset and kwargs[k] is not None} | ||
| # Remaining kwargs go to JSON body | ||
| payload = {k: v for k, v in kwargs.items() if v is not None} | ||
|
|
||
| # FIXED: Use param_serialize to properly construct the full URL with host | ||
| # Handle query parameters by appending them to the path | ||
| final_path = formatted_path | ||
| if query_params and http_method in ('GET', 'DELETE'): | ||
| query_string = '&'.join([f"{k}={v}" for k, v in query_params.items() if v is not None]) | ||
| if query_string: | ||
| separator = '&' if '?' in final_path else '?' | ||
| final_path = f"{final_path}{separator}{query_string}" | ||
|
|
||
| # Use param_serialize to get the proper URL with host | ||
| # Remove /api/v1 prefix from resource_path since host already includes it | ||
| resource_path_for_serialize = formatted_path.replace('/api/v1', '', 1) | ||
|
|
||
| method, url, header_params, serialized_body, post_params = self.api_client.param_serialize( | ||
| method=http_method, | ||
| resource_path=resource_path_for_serialize, # Use path without /api/v1 prefix | ||
| query_params=query_params or None, | ||
| header_params={}, | ||
| body=payload if http_method not in ('GET', 'DELETE') else None | ||
| ) | ||
|
|
||
| # Ensure required headers for PATCH/POST/PUT | ||
| if http_method not in ('GET', 'DELETE'): | ||
| header_params.setdefault('Content-Type', 'application/json') | ||
| header_params.setdefault('Accept', 'application/json') | ||
|
|
||
| # Add the authorization token to headers | ||
| original_call_api = self.api_client.call_api | ||
|
|
||
| def call_api_with_token(*args, **kwargs): | ||
| """Wrapper that adds authentication token to all API calls.""" | ||
| # Get the access token | ||
| token = self.token_manager.get_access_token() | ||
| header_params['Authorization'] = f"Bearer {token}" | ||
|
|
||
| # Call the REST client directly with the constructed URL | ||
| response = self.api_client.rest_client.request( | ||
| method=http_method, | ||
| url=url, | ||
| headers=header_params, | ||
| body=serialized_body if http_method not in ('GET', 'DELETE') else None, | ||
| post_params=post_params, | ||
| _request_timeout=None | ||
| ) | ||
|
|
||
| # Use the API client's response_deserialize to properly handle the response | ||
| # First read the response data | ||
| response.read() | ||
|
|
||
| # Then deserialize it | ||
| api_response = self.api_client.response_deserialize(response, self.RESPONSE_TYPES[resource][action]) | ||
| # Inject the token into headers | ||
| if 'header_params' not in kwargs: | ||
| kwargs['header_params'] = {} | ||
| kwargs['header_params']['Authorization'] = f"Bearer {token}" | ||
|
|
||
| return api_response.data | ||
| # Call the original method | ||
| return original_call_api(*args, **kwargs) | ||
|
|
||
| # Add docstring to the method based on the action and resource | ||
| if action == 'list': | ||
| docstring = f""" | ||
| Get a list of {resource}. | ||
| Args: | ||
| **kwargs: Optional arguments to pass to the API. | ||
| sort (str): Sort {resource} by field. (Optional) | ||
| page_size (int): Number of results per page. (Optional) | ||
| next_token (str): Token for the next page of results. (Optional) | ||
| Returns: | ||
| Dict containing {resource} data. | ||
| """ | ||
| elif action == 'get': | ||
| param_name = path.split('{')[-1].split('}')[0] if '{' in path else f"{resource_singular}_id" | ||
| docstring = f""" | ||
| Get a {resource_singular} by ID. | ||
| Args: | ||
| {param_name}: The ID of the {resource_singular} to get. | ||
| Returns: | ||
| Dict containing {resource_singular} data. | ||
| """ | ||
| elif action == 'create': | ||
| docstring = f""" | ||
| Create a new {resource_singular}. | ||
| Args: | ||
| **kwargs: {resource_singular.capitalize()} data to create. | ||
| # Replace the call_api method | ||
| self.api_client.call_api = call_api_with_token |
Contributor
There was a problem hiding this comment.
Add error handling for token acquisition and prevent silent header overwrites.
The token injection wrapper has several concerns:
- No error handling if
get_access_token()fails or returnsNone- this will result inAuthorization: Bearer Nonebeing sent. - Silently overwrites any existing Authorization header without checking or warning.
- Token fetch failures will propagate as unclear API errors rather than authentication errors.
🛡️ Proposed fix with error handling
def _setup_token_handling(self):
"""
Set up automatic token injection for all API calls.
This modifies the API client to automatically add the Bearer token
to all outgoing requests.
"""
original_call_api = self.api_client.call_api
def call_api_with_token(*args, **kwargs):
"""Wrapper that adds authentication token to all API calls."""
- # Get the access token
- token = self.token_manager.get_access_token()
+ # Get the access token with error handling
+ try:
+ token = self.token_manager.get_access_token()
+ if not token:
+ raise ValueError("Failed to acquire access token: token is empty")
+ except Exception as e:
+ logger.error(f"Token acquisition failed: {e}")
+ raise RuntimeError(f"Failed to authenticate with Kinde API: {e}") from e
# Inject the token into headers
if 'header_params' not in kwargs:
kwargs['header_params'] = {}
kwargs['header_params']['Authorization'] = f"Bearer {token}"
# Call the original method
return original_call_api(*args, **kwargs)
# Replace the call_api method
self.api_client.call_api = call_api_with_tokenCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @kinde_sdk/management/management_client.py around lines 100 - 123, The
token-injection wrapper in _setup_token_handling must handle failures from
token_manager.get_access_token() and avoid silently overwriting existing
Authorization headers: update call_api_with_token to try/except around
self.token_manager.get_access_token(), raise or return a clear
AuthenticationError (or raise ValueError) if the call raises or returns a falsy
token instead of injecting "Bearer None", and before setting
kwargs['header_params']['Authorization'] check for an existing Authorization
value and either preserve it or log/raise a warning so you don't overwrite it
silently; ensure the error you raise is descriptive (e.g., "Failed to acquire
access token") and replace api_client.call_api only after this robust wrapper is
implemented.
Comment on lines
125
to
152
| def _initialize_api_classes(self): | ||
| """ | ||
| Dynamically initialize all API classes from the api module. | ||
| This method inspects the api module and creates an instance of each | ||
| API class (classes ending with 'Api'), making them available as | ||
| snake_case attributes on this client. | ||
| For example: | ||
| - UsersApi -> self.users | ||
| - OrganizationsApi -> self.organizations | ||
| - FeatureFlagsApi -> self.feature_flags | ||
| """ | ||
| # Get all members of the api module | ||
| for name, obj in inspect.getmembers(api): | ||
| # Check if it's a class and ends with 'Api' | ||
| if inspect.isclass(obj) and name.endswith('Api'): | ||
| # Convert class name to snake_case attribute name | ||
| # e.g., UsersApi -> users, FeatureFlagsApi -> feature_flags | ||
| attr_name = self._class_name_to_snake_case(name) | ||
|
|
||
| Returns: | ||
| Dict containing the created {resource_singular}. | ||
| """ | ||
| elif action == 'update': | ||
| param_name = path.split('{')[-1].split('}')[0] if '{' in path else f"{resource_singular}_id" | ||
| docstring = f""" | ||
| Update a {resource_singular}. | ||
| Args: | ||
| {param_name}: The ID of the {resource_singular} to update. | ||
| **kwargs: {resource_singular.capitalize()} data to update. | ||
| # Initialize the API class with our configured api_client | ||
| api_instance = obj(api_client=self.api_client) | ||
|
|
||
| Returns: | ||
| Dict containing the updated {resource_singular}. | ||
| """ | ||
| elif action == 'delete': | ||
| param_name = path.split('{')[-1].split('}')[0] if '{' in path else f"{resource_singular}_id" | ||
| docstring = f""" | ||
| Delete a {resource_singular}. | ||
| Args: | ||
| {param_name}: The ID of the {resource_singular} to delete. | ||
| # Set it as an attribute on this client | ||
| setattr(self, attr_name, api_instance) | ||
|
|
||
| Returns: | ||
| Dict containing the result. | ||
| """ | ||
| else: | ||
| docstring = f""" | ||
| {action.capitalize()} {resource}. | ||
| logger.debug(f"Initialized {name} as client.{attr_name}") |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling and validation for dynamic API class loading.
The dynamic API initialization has several robustness concerns:
- No error handling if an API class fails to instantiate - this could happen if a class requires additional constructor parameters beyond
api_client. - No validation that
attr_namedoesn't collide with existing ManagementClient attributes (e.g.,domain,api_client,configuration). - Silent success if no API classes are found - the client would appear to work but have no methods.
- No verification that discovered classes are actually instantiable or are the expected API classes.
♻️ Proposed fix with validation and error handling
def _initialize_api_classes(self):
"""
Dynamically initialize all API classes from the api module.
This method inspects the api module and creates an instance of each
API class (classes ending with 'Api'), making them available as
snake_case attributes on this client.
For example:
- UsersApi -> self.users
- OrganizationsApi -> self.organizations
- FeatureFlagsApi -> self.feature_flags
"""
+ # Track loaded APIs for verification
+ loaded_apis = []
+
# Get all members of the api module
for name, obj in inspect.getmembers(api):
# Check if it's a class and ends with 'Api'
if inspect.isclass(obj) and name.endswith('Api'):
# Convert class name to snake_case attribute name
# e.g., UsersApi -> users, FeatureFlagsApi -> feature_flags
attr_name = self._class_name_to_snake_case(name)
+
+ # Check for attribute collisions
+ if hasattr(self, attr_name):
+ logger.warning(
+ f"Attribute '{attr_name}' already exists on ManagementClient, "
+ f"skipping {name}"
+ )
+ continue
- # Initialize the API class with our configured api_client
- api_instance = obj(api_client=self.api_client)
+ # Initialize the API class with error handling
+ try:
+ api_instance = obj(api_client=self.api_client)
+ except Exception as e:
+ logger.error(f"Failed to initialize {name}: {e}")
+ raise RuntimeError(
+ f"Failed to initialize API class {name}. "
+ f"This may indicate a compatibility issue with the generated API."
+ ) from e
# Set it as an attribute on this client
setattr(self, attr_name, api_instance)
+ loaded_apis.append(attr_name)
logger.debug(f"Initialized {name} as client.{attr_name}")
+
+ # Verify at least some APIs were loaded
+ if not loaded_apis:
+ raise RuntimeError(
+ "No API classes found in kinde_sdk.management.api module. "
+ "This may indicate the SDK was not properly installed or generated."
+ )
+
+ logger.info(f"Loaded {len(loaded_apis)} API classes: {', '.join(sorted(loaded_apis))}")
Comment on lines
185
to
495
| def get_users(self, **kwargs): | ||
| """ | ||
| Get users. | ||
| .. deprecated:: | ||
| Use :meth:`client.users.get_users()` instead. | ||
| For full documentation and parameters, see UsersApi.get_users() | ||
| """ | ||
| warnings.warn( | ||
| "get_users() is deprecated. Use client.users.get_users() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.users.get_users(**kwargs) | ||
|
|
||
| def get_user_data(self, user_id: str): | ||
| """ | ||
| Get user data by ID. | ||
| .. deprecated:: | ||
| Use :meth:`client.users.get_user_data()` instead. | ||
| For full documentation and parameters, see UsersApi.get_user_data() | ||
| """ | ||
| warnings.warn( | ||
| "get_user_data() is deprecated. Use client.users.get_user_data() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.users.get_user_data(user_id=user_id) | ||
|
|
||
| def create_user(self, create_user_request=None, **kwargs): | ||
| """ | ||
| Create a user. | ||
| .. deprecated:: | ||
| Use :meth:`client.users.create_user()` instead. | ||
| For full documentation and parameters, see UsersApi.create_user() | ||
| """ | ||
| warnings.warn( | ||
| "create_user() is deprecated. Use client.users.create_user() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.users.create_user(create_user_request=create_user_request, **kwargs) | ||
|
|
||
| def update_user(self, id: str, update_user_request, **kwargs): | ||
| """ | ||
| Update a user. | ||
| .. deprecated:: | ||
| Use :meth:`client.users.update_user()` instead. | ||
| For full documentation and parameters, see UsersApi.update_user() | ||
| """ | ||
| warnings.warn( | ||
| "update_user() is deprecated. Use client.users.update_user() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.users.update_user(id=id, update_user_request=update_user_request, **kwargs) | ||
|
|
||
| def delete_user(self, id: str, **kwargs): | ||
| """ | ||
| Delete a user. | ||
| .. deprecated:: | ||
| Use :meth:`client.users.delete_user()` instead. | ||
| For full documentation and parameters, see UsersApi.delete_user() | ||
| """ | ||
| warnings.warn( | ||
| "delete_user() is deprecated. Use client.users.delete_user() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.users.delete_user(id=id, **kwargs) | ||
|
|
||
| def get_organizations(self, **kwargs): | ||
| """ | ||
| Get organizations. | ||
| .. deprecated:: | ||
| Use :meth:`client.organizations.get_organizations()` instead. | ||
| For full documentation and parameters, see OrganizationsApi.get_organizations() | ||
| """ | ||
| warnings.warn( | ||
| "get_organizations() is deprecated. Use client.organizations.get_organizations() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.organizations.get_organizations(**kwargs) | ||
|
|
||
| def get_organization(self, code: str = None, **kwargs): | ||
| """ | ||
| Get an organization. | ||
| .. deprecated:: | ||
| Use :meth:`client.organizations.get_organization()` instead. | ||
| For full documentation and parameters, see OrganizationsApi.get_organization() | ||
| """ | ||
| warnings.warn( | ||
| "get_organization() is deprecated. Use client.organizations.get_organization() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.organizations.get_organization(code=code, **kwargs) | ||
|
|
||
| def create_organization(self, create_organization_request, **kwargs): | ||
| """ | ||
| Create an organization. | ||
| .. deprecated:: | ||
| Use :meth:`client.organizations.create_organization()` instead. | ||
| For full documentation and parameters, see OrganizationsApi.create_organization() | ||
| """ | ||
| warnings.warn( | ||
| "create_organization() is deprecated. Use client.organizations.create_organization() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.organizations.create_organization( | ||
| create_organization_request=create_organization_request, **kwargs | ||
| ) | ||
|
|
||
| def update_organization(self, org_code: str, **kwargs): | ||
| """ | ||
| Update an organization. | ||
| .. deprecated:: | ||
| Use :meth:`client.organizations.update_organization()` instead. | ||
| For full documentation and parameters, see OrganizationsApi.update_organization() | ||
| """ | ||
| warnings.warn( | ||
| "update_organization() is deprecated. Use client.organizations.update_organization() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.organizations.update_organization(org_code=org_code, **kwargs) | ||
|
|
||
| def delete_organization(self, org_code: str, **kwargs): | ||
| """ | ||
| Delete an organization. | ||
| .. deprecated:: | ||
| Use :meth:`client.organizations.delete_organization()` instead. | ||
| For full documentation and parameters, see OrganizationsApi.delete_organization() | ||
| """ | ||
| warnings.warn( | ||
| "delete_organization() is deprecated. Use client.organizations.delete_organization() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.organizations.delete_organization(org_code=org_code, **kwargs) | ||
|
|
||
| def get_roles(self, **kwargs): | ||
| """ | ||
| Get roles. | ||
| .. deprecated:: | ||
| Use :meth:`client.roles.get_roles()` instead. | ||
| For full documentation and parameters, see RolesApi.get_roles() | ||
| """ | ||
| warnings.warn( | ||
| "get_roles() is deprecated. Use client.roles.get_roles() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.roles.get_roles(**kwargs) | ||
|
|
||
| def get_role(self, role_id: str): | ||
| """ | ||
| Get a role. | ||
| .. deprecated:: | ||
| Use :meth:`client.roles.get_role()` instead. | ||
| For full documentation and parameters, see RolesApi.get_role() | ||
| """ | ||
| warnings.warn( | ||
| "get_role() is deprecated. Use client.roles.get_role() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.roles.get_role(role_id=role_id) | ||
|
|
||
| def create_role(self, create_role_request=None, **kwargs): | ||
| """ | ||
| Create a role. | ||
| .. deprecated:: | ||
| Use :meth:`client.roles.create_role()` instead. | ||
| For full documentation and parameters, see RolesApi.create_role() | ||
| """ | ||
| warnings.warn( | ||
| "create_role() is deprecated. Use client.roles.create_role() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.roles.create_role(create_role_request=create_role_request, **kwargs) | ||
|
|
||
| def update_role(self, role_id: str, update_role_request=None, **kwargs): | ||
| """ | ||
| Update a role. | ||
| .. deprecated:: | ||
| Use :meth:`client.roles.update_role()` instead. | ||
| For full documentation and parameters, see RolesApi.update_role() | ||
| """ | ||
| warnings.warn( | ||
| "update_role() is deprecated. Use client.roles.update_role() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.roles.update_role(role_id=role_id, update_role_request=update_role_request, **kwargs) | ||
|
|
||
| def delete_role(self, role_id: str): | ||
| """ | ||
| Delete a role. | ||
| .. deprecated:: | ||
| Use :meth:`client.roles.delete_role()` instead. | ||
| For full documentation and parameters, see RolesApi.delete_role() | ||
| """ | ||
| warnings.warn( | ||
| "delete_role() is deprecated. Use client.roles.delete_role() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.roles.delete_role(role_id=role_id) | ||
|
|
||
| def get_feature_flags(self, **kwargs): | ||
| """ | ||
| Get feature flags. | ||
| .. deprecated:: | ||
| Use :meth:`client.feature_flags.get_feature_flags()` instead. | ||
| For full documentation and parameters, see FeatureFlagsApi.get_feature_flags() | ||
| """ | ||
| warnings.warn( | ||
| "get_feature_flags() is deprecated. Use client.feature_flags.get_feature_flags() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.feature_flags.get_feature_flags(**kwargs) | ||
|
|
||
| def create_feature_flag(self, create_feature_flag_request=None, **kwargs): | ||
| """ | ||
| Create a feature flag. | ||
| .. deprecated:: | ||
| Use :meth:`client.feature_flags.create_feature_flag()` instead. | ||
| For full documentation and parameters, see FeatureFlagsApi.create_feature_flag() | ||
| """ | ||
| warnings.warn( | ||
| "create_feature_flag() is deprecated. Use client.feature_flags.create_feature_flag() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.feature_flags.create_feature_flag( | ||
| create_feature_flag_request=create_feature_flag_request, **kwargs | ||
| ) | ||
|
|
||
| def update_feature_flag(self, feature_flag_key: str, update_feature_flag_request=None, **kwargs): | ||
| """ | ||
| Update a feature flag. | ||
| .. deprecated:: | ||
| Use :meth:`client.feature_flags.update_feature_flag()` instead. | ||
| For full documentation and parameters, see FeatureFlagsApi.update_feature_flag() | ||
| """ | ||
| warnings.warn( | ||
| "update_feature_flag() is deprecated. Use client.feature_flags.update_feature_flag() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.feature_flags.update_feature_flag( | ||
| feature_flag_key=feature_flag_key, | ||
| update_feature_flag_request=update_feature_flag_request, | ||
| **kwargs | ||
| ) | ||
|
|
||
| def delete_feature_flag(self, feature_flag_key: str): | ||
| """ | ||
| Delete a feature flag. | ||
| .. deprecated:: | ||
| Use :meth:`client.feature_flags.delete_feature_flag()` instead. | ||
| For full documentation and parameters, see FeatureFlagsApi.delete_feature_flag() | ||
| """ | ||
| warnings.warn( | ||
| "delete_feature_flag() is deprecated. Use client.feature_flags.delete_feature_flag() instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return self.feature_flags.delete_feature_flag(feature_flag_key=feature_flag_key) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find kinde_sdk/management/api -type f -name "*.py" | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 129
🏁 Script executed:
ls -la kinde_sdk/management/Repository: kinde-oss/kinde-python-sdk
Length of output: 965
🏁 Script executed:
rg "class.*Api" kinde_sdk/management/api --type py -B 2 | head -100Repository: kinde-oss/kinde-python-sdk
Length of output: 134
🏁 Script executed:
rg "class (UsersApi|OrganizationsApi|RolesApi|FeatureFlagsApi)" --type pyRepository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
head -100 kinde_sdk/management/__init__.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 6907
🏁 Script executed:
rg "UsersApi|OrganizationsApi|RolesApi|FeatureFlagsApi" kinde_sdk/management/management_client.py -B 5 -A 5Repository: kinde-oss/kinde-python-sdk
Length of output: 9788
🏁 Script executed:
find . -path "./kinde_sdk/management/api/*.py" -type f | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
ls -la kinde_sdk/management/api/ 2>/dev/null || echo "Checking alternative locations..."Repository: kinde-oss/kinde-python-sdk
Length of output: 103
🏁 Script executed:
rg "def create_user|def update_user|def get_users" --type py -A 10 | head -150Repository: kinde-oss/kinde-python-sdk
Length of output: 3630
🏁 Script executed:
find . -name "*.py" -type f -exec grep -l "class UsersApi" {} \;Repository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
git ls-files | grep -i api | head -30Repository: kinde-oss/kinde-python-sdk
Length of output: 1251
🏁 Script executed:
ls -la kinde_sdk/Repository: kinde-oss/kinde-python-sdk
Length of output: 582
🏁 Script executed:
cat .openapi-generator/FILES | grep -i "api" | head -40Repository: kinde-oss/kinde-python-sdk
Length of output: 1636
🏁 Script executed:
ls -la .openapi-generator/Repository: kinde-oss/kinde-python-sdk
Length of output: 301
🏁 Script executed:
grep -r "class.*Api" kinde_sdk/ --type py | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 187
🏁 Script executed:
grep -n "def create_user\|def update_user\|def get_users" kinde_sdk/apis/tags/users_api.py -A 15Repository: kinde-oss/kinde-python-sdk
Length of output: 135
🏁 Script executed:
grep -n "def create_organization\|def update_organization" kinde_sdk/apis/tags/organizations_api.py -A 15Repository: kinde-oss/kinde-python-sdk
Length of output: 143
🏁 Script executed:
grep -n "def create_role\|def update_role" kinde_sdk/apis/tags/roles_api.py -A 15Repository: kinde-oss/kinde-python-sdk
Length of output: 135
🏁 Script executed:
grep -n "def create_feature_flag\|def update_feature_flag" kinde_sdk/apis/tags/feature_flags_api.py -A 15Repository: kinde-oss/kinde-python-sdk
Length of output: 143
🏁 Script executed:
ls -la kinde_sdk/apis/tags/ 2>/dev/null | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
find kinde_sdk -name "users_api.py" -type fRepository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
pwd && git statusRepository: kinde-oss/kinde-python-sdk
Length of output: 352
🏁 Script executed:
git sparse-checkout listRepository: kinde-oss/kinde-python-sdk
Length of output: 1374
🏁 Script executed:
git ls-files | grep "api" | wc -lRepository: kinde-oss/kinde-python-sdk
Length of output: 73
🏁 Script executed:
cat kinde_sdk/management/__init__.py | head -50Repository: kinde-oss/kinde-python-sdk
Length of output: 2737
🏁 Script executed:
wc -l kinde_sdk/management/management_client.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 115
🏁 Script executed:
head -100 kinde_sdk/management/management_client.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 3608
🏁 Script executed:
sed -n '1,184p' kinde_sdk/management/management_client.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 6933
🏁 Script executed:
sed -n '184,250p' kinde_sdk/management/management_client.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 2238
🏁 Script executed:
grep -n "def " kinde_sdk/management/management_client.py | tail -30Repository: kinde-oss/kinde-python-sdk
Length of output: 1509
🏁 Script executed:
cat kinde_sdk/management/management_client.py | sed -n '185,495p' > /tmp/deprecated_methods.txt && wc -l /tmp/deprecated_methods.txtRepository: kinde-oss/kinde-python-sdk
Length of output: 101
🏁 Script executed:
find . -name "*.py" -type f -path "*/test*" -o -path "*/example*" | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 842
🏁 Script executed:
grep -r "create_user\|update_user" --include="*.py" | grep -E "(test|example)" | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 2106
🏁 Script executed:
grep -n "pipeline\|failed\|error" kinde_sdk/management/management_client.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
cat testv2/testv2_management/test_management_client.py | head -200Repository: kinde-oss/kinde-python-sdk
Length of output: 9100
🏁 Script executed:
cat testv2/testv2_management/test_management_client.py | grep -A 30 "test_create_user_with_body_data"Repository: kinde-oss/kinde-python-sdk
Length of output: 1655
🏁 Script executed:
cat examples/management_client_example.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 11360
🏁 Script executed:
python3 << 'EOF'
import re
# Read the management_client.py file
with open('kinde_sdk/management/management_client.py', 'r') as f:
content = f.read()
# Extract all deprecated wrapper methods
pattern = r'def (\w+)\(self(?:, ([^)]*))?\):'
matches = re.findall(pattern, content)
# Focus on the deprecated methods (lines 185+)
deprecated_start = content.find('def get_users(self')
deprecated_section = content[deprecated_start:]
# Count methods and analyze kwargs patterns
kwargs_methods = []
for match in re.finditer(r'def (\w+)\(self([^)]*)\):', deprecated_section):
method_name = match.group(1)
params = match.group(2)
if '**kwargs' in params:
kwargs_methods.append((method_name, params.strip()))
print(f"Total deprecated methods with **kwargs: {len(kwargs_methods)}")
print("\nMethods and their signatures:")
for method, params in kwargs_methods[:10]:
print(f" {method}({params})")
EOFRepository: kinde-oss/kinde-python-sdk
Length of output: 576
🌐 Web query:
Kinde Python SDK OpenAPI generated API kwargs handling
💡 Result:
Short answer: Kinde’s Python SDK uses OpenAPI-generated client methods that accept standard OpenAPI-generator **kwargs (e.g. async_req, _preload_content, _return_http_data_only, _request_timeout, _request_auth). Pass these kwargs to control async behavior, timeouts, response form, and per-request auth; the SDK’s higher-level ManagementClient/clients simply call those generated methods under the hood. [1][2][3]
Common kwargs (supported by OpenAPI‑generated clients)
- async_req=True — run request in background thread and return a thread/future. [2]
- _return_http_data_only=True/False — return only parsed data vs (data, status, headers). [2]
- _preload_content=False — return raw urllib3.HTTPResponse without decoding. [2]
- _request_timeout=(connect_timeout, read_timeout) or single number. [2]
- _request_auth=dict — override auth for that call. [2]
- Any endpoint-specific optional parameters are passed as named args or in **kwargs.
Example (concise)
- Synchronous, only data:
client.some_api_call(required1=val, optional2=val, _return_http_data_only=True) - Async with timeout:
thread = client.some_api_call(required1=val, async_req=True, _request_timeout=(3, 10))
result = thread.get()
Notes
- The Kinde SDK exposes higher-level async helpers; consult the SDK docs if you prefer their async/await wrappers. [3]
- If you need custom headers or auth per call, use the generated client kwargs (_request_auth) or set headers on the ApiClient instance. [2]
Sources
- Kinde Python SDK (GitHub). [1]
- OpenAPI‑generated Python client method kwargs documentation (example: kubernetes/openapi-generated docs). [2]
- Kinde Python SDK docs (Python SDK / Management API usage). [3]
Add type hints to request parameter defaults for clarity.
Several request parameters default to None without proper type annotations:
create_user(self, create_user_request=None, **kwargs)create_organization(self, create_organization_request, **kwargs)create_role(self, create_role_request=None, **kwargs)create_feature_flag(self, create_feature_flag_request=None, **kwargs)update_user(self, id: str, update_user_request, **kwargs)update_role(self, role_id: str, update_role_request=None, **kwargs)update_feature_flag(self, feature_flag_key: str, update_feature_flag_request=None, **kwargs)
Import the request model types and annotate: create_user_request: Optional[CreateUserRequest] = None.
The **kwargs forwarding is correct—OpenAPI-generated methods are designed to accept standard kwargs (e.g., async_req, _request_timeout, _return_http_data_only).
🧰 Tools
🪛 GitHub Actions: CI
[error] 231-231: pydantic.ValidationError: create_user received unexpected keyword arguments (email, given_name, family_name) when calling UsersApi.create_user.
🪛 Ruff (0.14.10)
281-281: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🤖 Prompt for AI Agents
In @kinde_sdk/management/management_client.py around lines 185 - 495, Add
explicit Optional[type] annotations for parameters that default to None: import
typing.Optional and the corresponding request models (e.g., CreateUserRequest,
CreateOrganizationRequest, CreateRoleRequest, CreateFeatureFlagRequest,
UpdateUserRequest, UpdateRoleRequest, UpdateFeatureFlagRequest) and update
signatures such as create_user(create_user_request: Optional[CreateUserRequest]
= None, **kwargs), create_organization(create_organization_request:
Optional[CreateOrganizationRequest] = None, **kwargs),
create_role(create_role_request: Optional[CreateRoleRequest] = None, **kwargs),
create_feature_flag(create_feature_flag_request:
Optional[CreateFeatureFlagRequest] = None, **kwargs), update_user(id: str,
update_user_request: Optional[UpdateUserRequest] = None, **kwargs),
update_role(role_id: str, update_role_request: Optional[UpdateRoleRequest] =
None, **kwargs), and update_feature_flag(feature_flag_key: str,
update_feature_flag_request: Optional[UpdateFeatureFlagRequest] = None,
**kwargs); keep the existing **kwargs forwarding behavior unchanged.
| ) | ||
| return self.organizations.get_organizations(**kwargs) | ||
|
|
||
| def get_organization(self, code: str = None, **kwargs): |
Contributor
There was a problem hiding this comment.
Fix implicit Optional type hint.
PEP 484 requires explicit Optional or union syntax when a parameter defaults to None.
🔧 Proposed fix
- def get_organization(self, code: str = None, **kwargs):
+ def get_organization(self, code: Optional[str] = None, **kwargs):Or using Python 3.10+ union syntax:
- def get_organization(self, code: str = None, **kwargs):
+ def get_organization(self, code: str | None = None, **kwargs):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def get_organization(self, code: str = None, **kwargs): | |
| def get_organization(self, code: Optional[str] = None, **kwargs): |
Suggested change
| def get_organization(self, code: str = None, **kwargs): | |
| def get_organization(self, code: str | None = None, **kwargs): |
🧰 Tools
🪛 Ruff (0.14.10)
281-281: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🤖 Prompt for AI Agents
In @kinde_sdk/management/management_client.py at line 281, The parameter type
for get_organization should explicitly allow None; update the signature of
get_organization to use an explicit Optional type (e.g., Optional[str] or Python
3.10+ union syntax str | None) instead of relying on a default None, and add the
corresponding import (from typing import Optional) if using Optional to satisfy
PEP 484.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.gitignore:
- Line 45: The .gitignore entry "*,cover" is likely a typo using a comma;
replace that pattern with the intended wildcard extension (e.g., "*.cover" or
the correct filename/pattern you want ignored) so the file ignores files with
the .cover extension rather than a literal "*,cover" entry; update the
.gitignore line that currently reads "*,cover" to use the correct glob pattern
"*.cover" (or the appropriate pattern for the coverage file you intend to
exclude).
In `@kinde_sdk/management/management_client.py`:
- Around line 139-142: The inline comment in the block that checks
inspect.isclass(obj) and calls self._class_name_to_snake_case(name) is
misleading: update the examples to show that Api classes produce attributes with
the "_api" suffix (e.g., UsersApi -> users_api, FeatureFlagsApi ->
feature_flags_api) so it accurately reflects how _class_name_to_snake_case(name)
is used to build attribute names; modify the comment near that
inspect.isclass(...) check and the reference to attr_name to mention the "_api"
suffix.
- Around line 55-72: The example uses a non-existent billing_api namespace which
causes confusion; update the usage in the example for ManagementClient to
reference a real generated API (e.g., replace billing_api.get_billing_info()
with a known namespace such as roles_api.get_roles() or
organizations_api.get_organizations()) so the doc demonstrates a valid dynamic
API access; change the final line in the example block to call an actual API on
the ManagementClient instance (refer to ManagementClient, billing_api,
roles_api, organizations_api, users_api in the patch to locate and update the
example).
- Around line 197-201: Update the documentation comment for the
backwards-compatibility shortcuts to reference the correct public attribute
name: change any example usage that shows client.users.get_users() to use
client.users_api.get_users() (and similarly for other examples), ensuring the
comment consistently mentions users_api as the public attribute that delegates
to the Users API methods.
In `@testv2/testv2_management/test_management_client.py`:
- Around line 245-248: The test is calling get_users(user_id=...) which treats
user_id as a query filter, but you want to test the /users/{id} path; change the
call to use get_user_data(id=user_id) (replace the get_users invocation with
get_user_data and pass the same user_id as the required id parameter) and update
any variable names/assertions if they depend on the previous response shape to
match get_user_data's output.
♻️ Duplicate comments (3)
kinde_sdk/management/management_client.py (3)
98-118: Token wrapper still needs error handling and header preservation.
Access-token failures can lead toBearer None, and Authorization is overwritten unconditionally.🛡️ Suggested hardening
- # Get the access token - token = self.token_manager.get_access_token() + try: + token = self.token_manager.get_access_token() + if not token: + raise ValueError("Failed to acquire access token: token is empty") + except Exception as exc: + logger.error("Token acquisition failed: %s", exc) + raise RuntimeError("Failed to acquire access token") from exc # Inject the token into headers if 'header_params' not in kwargs: kwargs['header_params'] = {} + if 'Authorization' in kwargs['header_params']: + logger.warning("Overriding existing Authorization header") kwargs['header_params']['Authorization'] = f"Bearer {token}"
123-150: Dynamic API loading needs collision and instantiation guards.
There’s no protection against attribute collisions or constructor failures, and an empty API set would be silent.♻️ Suggested safeguards
- for name, obj in inspect.getmembers(api): + loaded_apis = [] + for name, obj in inspect.getmembers(api): if inspect.isclass(obj) and name.endswith('Api'): attr_name = self._class_name_to_snake_case(name) + if hasattr(self, attr_name): + logger.warning("Attribute '%s' already exists; skipping %s", attr_name, name) + continue - api_instance = obj(api_client=self.api_client) + try: + api_instance = obj(api_client=self.api_client) + except Exception as exc: + logger.error("Failed to initialize %s: %s", name, exc) + raise RuntimeError(f"Failed to initialize API class {name}") from exc setattr(self, attr_name, api_instance) + loaded_apis.append(attr_name) logger.debug(f"Initialized {name} as client.{attr_name}") + if not loaded_apis: + raise RuntimeError("No API classes found in kinde_sdk.management.api module.")
314-328: Explicit Optional for code parameter.
PEP 484 requires explicit Optional (or union) when the default is None.🔧 Suggested fix
- def get_organization(self, code: str = None, **kwargs): + def get_organization(self, code: str | None = None, **kwargs):
🧹 Nitpick comments (4)
generate_management_sdk.py (3)
305-305: Consider movingimport jsonto the top of the file.The
jsonmodule is imported inside the function. While this works, it's more conventional to import standard library modules at the top of the file for better visibility and consistency.
400-407: Unused variableresultfrom subprocess call.The
resultvariable captures the subprocess output but is never used. Consider using it for logging or removing the assignment.♻️ Option 1: Use underscore to indicate intentionally unused
- result = subprocess.run(cmd, check=True, capture_output=True, text=True) + subprocess.run(cmd, check=True, capture_output=True, text=True)♻️ Option 2: Log the output for debugging purposes
result = subprocess.run(cmd, check=True, capture_output=True, text=True) print("✓ Generation completed successfully") + if result.stdout: + logger.debug(result.stdout)
553-557: Fragile index access toconfig["custom_imports"][1].The code assumes the first element is an empty line and accesses index
[1]directly. If the config structure changes, this could break. Consider using a more robust check.♻️ Consider a more robust presence check
- # Check if custom imports are already present (check first import line) - first_custom_import = config["custom_imports"][1] # Skip empty line - if first_custom_import.lstrip('# ') in content: + # Check if custom imports are already present + custom_import_marker = "# Custom imports for Kinde Management Client" + if custom_import_marker in content:kinde_sdk/management/schemas.py (1)
21-21: Unusednoqa: F401directive.The static analysis tool indicates the noqa directive for F401 is not needed on this line. The imports (date, datetime, timedelta) are used in the file.
♻️ Remove unused noqa directive
-from datetime import date, datetime, timedelta # noqa: F401 +from datetime import date, datetime, timedelta
| nosetests.xml | ||
| coverage.xml | ||
| *.cover | ||
| *,cover |
Contributor
There was a problem hiding this comment.
Potential typo: *,cover should likely be *.cover.
The pattern *,cover uses a comma instead of a period, which is unusual. Python coverage files typically have the .cover extension.
🔧 Proposed fix
-*,cover
+*.cover📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| *,cover | |
| *.cover |
🤖 Prompt for AI Agents
In @.gitignore at line 45, The .gitignore entry "*,cover" is likely a typo using
a comma; replace that pattern with the intended wildcard extension (e.g.,
"*.cover" or the correct filename/pattern you want ignored) so the file ignores
files with the .cover extension rather than a literal "*,cover" entry; update
the .gitignore line that currently reads "*,cover" to use the correct glob
pattern "*.cover" (or the appropriate pattern for the coverage file you intend
to exclude).
Comment on lines
+55
to
+72
| Example: | ||
| ```python | ||
| client = ManagementClient(domain, client_id, client_secret) | ||
| # Get all users | ||
| users = client.users_api.get_users(page_size=50) | ||
| # Create a user | ||
| new_user = client.users_api.create_user( | ||
| create_user_request={'email': 'user@example.com'} | ||
| ) | ||
| # Get organizations | ||
| orgs = client.organizations_api.get_organizations() | ||
| # Access any dynamically loaded API | ||
| billing = client.billing_api.get_billing_info() | ||
| ``` |
Contributor
There was a problem hiding this comment.
*Doc example should reference an existing _api namespace.
billing_api doesn’t appear to be part of the generated management APIs anymore; consider switching the example to a known namespace to avoid confusion.
✏️ Suggested doc update
- billing = client.billing_api.get_billing_info()
+ timezones = client.timezones_api.get_timezones()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Example: | |
| ```python | |
| client = ManagementClient(domain, client_id, client_secret) | |
| # Get all users | |
| users = client.users_api.get_users(page_size=50) | |
| # Create a user | |
| new_user = client.users_api.create_user( | |
| create_user_request={'email': 'user@example.com'} | |
| ) | |
| # Get organizations | |
| orgs = client.organizations_api.get_organizations() | |
| # Access any dynamically loaded API | |
| billing = client.billing_api.get_billing_info() | |
| ``` | |
| Example: |
🤖 Prompt for AI Agents
In `@kinde_sdk/management/management_client.py` around lines 55 - 72, The example
uses a non-existent billing_api namespace which causes confusion; update the
usage in the example for ManagementClient to reference a real generated API
(e.g., replace billing_api.get_billing_info() with a known namespace such as
roles_api.get_roles() or organizations_api.get_organizations()) so the doc
demonstrates a valid dynamic API access; change the final line in the example
block to call an actual API on the ManagementClient instance (refer to
ManagementClient, billing_api, roles_api, organizations_api, users_api in the
patch to locate and update the example).
Comment on lines
+139
to
+142
| if inspect.isclass(obj) and name.endswith('Api'): | ||
| # Convert class name to snake_case attribute name | ||
| # e.g., UsersApi -> users, FeatureFlagsApi -> feature_flags | ||
| attr_name = self._class_name_to_snake_case(name) |
Contributor
There was a problem hiding this comment.
Inline comment should mention users_api/feature_flags_api.
The comment currently says UsersApi -> users, but the actual attribute is users_api.
✏️ Suggested tweak
- # e.g., UsersApi -> users, FeatureFlagsApi -> feature_flags
+ # e.g., UsersApi -> users_api, FeatureFlagsApi -> feature_flags_api🤖 Prompt for AI Agents
In `@kinde_sdk/management/management_client.py` around lines 139 - 142, The inline
comment in the block that checks inspect.isclass(obj) and calls
self._class_name_to_snake_case(name) is misleading: update the examples to show
that Api classes produce attributes with the "_api" suffix (e.g., UsersApi ->
users_api, FeatureFlagsApi -> feature_flags_api) so it accurately reflects how
_class_name_to_snake_case(name) is used to build attribute names; modify the
comment near that inspect.isclass(...) check and the reference to attr_name to
mention the "_api" suffix.
Comment on lines
+197
to
+201
| # Backwards compatibility: Provide direct method access for common operations | ||
| # These delegate to the appropriate API class methods | ||
| # Note: These methods are deprecated. For full functionality and proper type hints, | ||
| # use the API class methods directly (e.g., client.users.get_users()) | ||
|
|
Contributor
There was a problem hiding this comment.
Doc note should reference client.users_api, not client.users.
The public attribute is users_api, so the example in the comment should match.
✏️ Suggested tweak
- # use the API class methods directly (e.g., client.users.get_users())
+ # use the API class methods directly (e.g., client.users_api.get_users())🤖 Prompt for AI Agents
In `@kinde_sdk/management/management_client.py` around lines 197 - 201, Update the
documentation comment for the backwards-compatibility shortcuts to reference the
correct public attribute name: change any example usage that shows
client.users.get_users() to use client.users_api.get_users() (and similarly for
other examples), ensuring the comment consistently mentions users_api as the
public attribute that delegates to the Users API methods.
Comment on lines
245
to
248
| # Test API call with path parameter | ||
| user_id = "user123" | ||
| result = client.get_user(user_id) | ||
| result = client.get_users(user_id=user_id) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the signature of get_users in UsersApi
rg -n "def get_users" -A 6 -B 2Repository: kinde-oss/kinde-python-sdk
Length of output: 7440
🏁 Script executed:
# Look at the test code around lines 245-248
cat -n testv2/testv2_management/test_management_client.py | sed -n '240,255p'Repository: kinde-oss/kinde-python-sdk
Length of output: 773
🏁 Script executed:
# Check if get_user_data method exists anywhere in the codebase
rg -n "def get_user_data" --type pyRepository: kinde-oss/kinde-python-sdk
Length of output: 501
🏁 Script executed:
# Look at the broader test context to understand what's being tested
cat -n testv2/testv2_management/test_management_client.py | sed -n '235,260p'Repository: kinde-oss/kinde-python-sdk
Length of output: 1391
🏁 Script executed:
# Check get_user_data method in management_client.py
cat -n kinde_sdk/management/management_client.py | sed -n '230,250p'Repository: kinde-oss/kinde-python-sdk
Length of output: 906
🏁 Script executed:
# Check the endpoint that get_users calls (look for request_path)
rg -A 30 "def get_users" kinde_sdk/management/api/users_api.py | grep -E "(request_path|local_var_path)" | head -5Repository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
# Check the endpoint that get_user_data calls
rg -A 30 "def get_user_data" kinde_sdk/management/api/users_api.py | grep -E "(request_path|local_var_path)" | head -5Repository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
# Look at broader context of get_users implementation
cat -n kinde_sdk/management/api/users_api.py | sed -n '2340,2400p'Repository: kinde-oss/kinde-python-sdk
Length of output: 4907
🏁 Script executed:
# Look at get_user_data implementation context
cat -n kinde_sdk/management/api/users_api.py | sed -n '1202,1270p'Repository: kinde-oss/kinde-python-sdk
Length of output: 3638
Use get_user_data() for the /users/{id} path test.
The test comment indicates a path parameter test, but get_users(user_id=...) uses user_id as an optional query filter, not a path parameter. For retrieving a single user by ID via the /users/{id} path, use get_user_data() instead, which takes id as a required path parameter.
🔧 Suggested fix
- result = client.get_users(user_id=user_id)
+ result = client.get_user_data(user_id)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Test API call with path parameter | |
| user_id = "user123" | |
| result = client.get_user(user_id) | |
| result = client.get_users(user_id=user_id) | |
| # Test API call with path parameter | |
| user_id = "user123" | |
| result = client.get_user_data(user_id) | |
🤖 Prompt for AI Agents
In `@testv2/testv2_management/test_management_client.py` around lines 245 - 248,
The test is calling get_users(user_id=...) which treats user_id as a query
filter, but you want to test the /users/{id} path; change the call to use
get_user_data(id=user_id) (replace the get_users invocation with get_user_data
and pass the same user_id as the required id parameter) and update any variable
names/assertions if they depend on the previous response shape to match
get_user_data's output.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explain your changes
This updates how the management client is initialised.
Instead of defining each route, it handles directly from the generated code.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.