Fix/pytest conditional deps #144
Conversation
- Update response types for various endpoints - Add new endpoints for user identities management
Fix the singularization bug that was generating incorrect method names: - create_user_identitie → create_user_identity - get_identitie → get_identity - update_identitie → update_identity - delete_identitie → delete_identity Changes: - Add _singularize_resource() helper method to centralize singularization logic - Handle special cases: user_identities → user_identity, identities → identity - Update both _generate_methods() and _create_api_method() to use the helper - Maintains existing behavior for other resources (users, organizations, etc.) This fixes a pre-existing bug that was exposed by PR kinde-oss#128's addition of user_identities and identities endpoints.
Update test assertions to reflect removed endpoints: - Remove get_permission assertion (permissions no longer has get endpoint) - Remove get_feature_flags and get_feature_flag assertions (removed in PR kinde-oss#128) - Remove update_api_application assertion (removed in PR kinde-oss#128) - Update test_feature_flags_api_calls to test create_feature_flag instead These changes align tests with the updated Management API endpoints introduced in PR kinde-oss#128.
Add the missing 'list' endpoint to feature_flags resource to enable
get_feature_flags() method generation.
Changes:
- Add 'list': ('GET', '/api/v1/feature_flags') to feature_flags API_ENDPOINTS
- Add corresponding response type mapping: 'list': {'200': 'GetFeatureFlagsResponse', ...}
- Update test to assert get_feature_flags method exists
This addresses the CodeRabbit review comment on PR kinde-oss#143 and enables
the SDK to generate the get_feature_flags() method as documented in
the Kinde Management API.
…nse types - Corrected endpoint paths for organizations and API applications to use plural forms. - Updated response types for create endpoints to reflect correct HTTP status codes (201). - Adjusted test assertions to ensure alignment with the updated API structure. These changes enhance consistency with the Kinde Management API specifications.
Implement conditional pytest dependencies to maintain backward compatibility with Python 3.9 while allowing Python 3.10+ to use pytest v9. Changes: - Add environment markers in pyproject.toml [project.optional-dependencies.dev] - Python 3.9: pytest>=7.4.0,<9.0.0 - Python 3.10+: pytest>=9.0.0 - Update requirements.txt with conditional pytest dependencies using markers - Update pytest-asyncio to ^0.26.0 and black to ^24.3.0 (from PR kinde-oss#134) - Keep Poetry dev-dependencies with Python 3.9 compatible version as fallback This resolves the Python 3.9 compatibility issue in PR kinde-oss#134 where pytest v9 requires Python 3.10+. The conditional dependencies ensure: - Python 3.9 CI/CD continues to work with pytest <9.0.0 - Python 3.10+ developers can use pytest v9 features - No breaking changes for existing Python 3.9 users Fixes: kinde-oss#134
WalkthroughCentralized plural-to-singular resource handling in ManagementClient, added dynamic API method generation, expanded endpoints/response mappings (including identities), adjusted many endpoint paths and HTTP verbs, updated development dependency constraints in requirements, and modified management client tests to match the new API surface. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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
🧹 Nitpick comments (1)
testv2/testv2_management/test_management_client.py (1)
489-531: Update test method name to reflect the changed test case.The test has been changed from testing
get_feature_flags(GET) to testingcreate_feature_flag(POST), which is a completely different operation. The test method nametest_feature_flags_api_callsis now misleading.Consider renaming the test method:
- def test_feature_flags_api_calls(self, mock_token_manager_class, mock_api_client_class, mock_config_class): - """Test feature flag-related API calls.""" + def test_create_feature_flag(self, mock_token_manager_class, mock_api_client_class, mock_config_class): + """Test creating a feature flag."""Additionally, consider adding a separate test for
get_feature_flagsif that functionality still exists and needs testing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pyproject.tomlis excluded by!**/*.toml
📒 Files selected for processing (3)
kinde_sdk/management/management_client.py(12 hunks)requirements.txt(1 hunks)testv2/testv2_management/test_management_client.py(2 hunks)
🧰 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:
testv2/testv2_management/test_management_client.py
🔇 Additional comments (4)
kinde_sdk/management/management_client.py (2)
208-216: LGTM! New identities endpoints follow REST conventions.The new user_identities and identities endpoints are well-structured:
user_identitiesprovides user-scoped list and create operationsidentitiesprovides identity-scoped get, update, and delete operationsThis separation aligns with REST best practices.
88-90: Based on my verification:Rewritten Review Comment:
Confirm HTTP verb changes align with Kinde Management API specification.
The update operations have been changed from PATCH to PUT:
- Line 89:
feature_flagsupdate to PUT- Line 131:
propertiesupdate to PUTVerification finds that the
propertiesendpoint change to PUT aligns with Kinde's documented API specification (which notes a prior PUT/PATCH mismatch was fixed). However, thefeature_flagsendpoint's PUT method is not explicitly documented in Kinde's public API documentation—only the SDK helper methods are documented. Please confirm:
- The
feature_flagsPUT endpoint is supported by Kinde's Management API- These changes are intentional corrections to the SDK (not regressions)
- Any migration guidance or breaking change notes for downstream users
requirements.txt (2)
29-32: Black 24.3.0 supports Python 3.8+, making the version update safe.Verification confirms black 24.3.0 requires Python ≥ 3.8, which exceeds the Python 3.9 support threshold mentioned in the review. Additionally, pytest-asyncio 0.26.0 requires Python ≥ 3.9. Both version updates are compatible with the project's Python requirements.
23-27: No compatibility issues found; the conditional pytest dependencies are correct and compatible with pytest-asyncio 0.26.0.The web search confirms:
- pytest 9.0.0 requires Python ≥ 3.10 ✓
- pytest-asyncio 0.26.0 requires pytest ≥ 5.4.0 (no upper bound) ✓
- Both pytest 7.4.x and pytest 9.0.0+ satisfy pytest-asyncio's requirements
The conditional dependency structure properly handles the Python version constraint for pytest 9.0.0. No changes needed.
| 'list': {'200': 'UsersResponse', '400': 'ErrorResponse', '403': 'ErrorResponse', '429': 'ErrorResponse'}, | ||
| 'get': {'200': 'User', '400': 'ErrorResponse', '403': 'ErrorResponse', '404': 'ErrorResponse', '429': 'ErrorResponse'}, | ||
| 'create': {'201': 'CreateUserResponse', '400': 'ErrorResponse', '403': 'ErrorResponse', '429': 'ErrorResponse'}, | ||
| 'create': {'200': 'CreateUserResponse', '400': 'ErrorResponse', '403': 'ErrorResponse', '429': 'ErrorResponse'}, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
[run_scripts]Repository: kinde-oss/kinde-python-sdk
Length of output: 196
🌐 Web query:
Kinde Management API documentation HTTP status codes create update
💡 Result:
Do you want a short list of the HTTP status codes Kinde’s Management API returns for create/update operations (and common error codes), or the exact per-endpoint documented responses?
🌐 Web query:
Kinde Management API users create endpoint HTTP 201 or 200 response
💡 Result:
The Kinde Management API's POST /api/v1/user (create user) returns 201 Created on success (not 200). (docs.kinde.com)
Citations:
🌐 Web query:
Kinde Management API roles update endpoint HTTP response status code
💡 Result:
Which exact endpoint do you mean? (e.g. PATCH /api/v1/roles/{role_id}/permissions or PATCH /api/v1/roles/{role_id})
🌐 Web query:
Kinde Management API PATCH /api/v1/roles/{role_id} HTTP response status code 200 or 201
💡 Result:
PATCHing an existing Kinde role returns 200 OK (201 is for newly created resources, not updates). [1][2]
Sources:
- Kinde — Manage user roles (describes editing roles via the Management API). [1]
- Kinde — Kinde APIs / Management API reference (Management API used for role updates). [2]
🌐 Web query:
Kinde Management API webhooks create POST endpoint HTTP response status code 201 or 200
💡 Result:
Kinde’s docs don’t state an explicit HTTP status for the Management API POST /api/v1/webhooks create endpoint. The webhooks docs list the endpoint but give no response-code sample; other Kinde Management API create examples in the docs/community show 200 for resource-creation calls. (docs.kinde.com)
Recommendation: treat either 200 or 201 as possible—detect which your Kinde instance returns by making a test POST (or check the actual response in your client). If you need strict behavior (e.g., for client code), implement acceptance for both 200 and 201.
Citations:
Based on my verification through the Kinde Management API documentation, I can now provide the rewritten review comment:
HTTP status codes for create and update operations are incorrect.
The response type mappings have incorrect status codes that violate REST conventions:
- Line 224 –
userscreate endpoint should return201 Created(not200). Per Kinde docs, POST /api/v1/user returns 201. - Line 255 –
rolesupdate endpoint should return200 OK(not201). Per Kinde docs, PATCH /api/v1/roles/{role_id} returns 200.
Line 310 (webhooks create) returns 200, which may align with Kinde's non-standard pattern for some create operations, so that line does not require correction.
🤖 Prompt for AI Agents
In kinde_sdk/management/management_client.py around line 224 and line 255, the
HTTP status code mappings for the users create and roles update endpoints are
incorrect: change the users create mapping at line 224 from '200' to '201' (POST
/api/v1/user returns 201 Created) and change the roles update mapping at line
255 from '201' to '200' (PATCH /api/v1/roles/{role_id} returns 200 OK); update
only the status code keys in the response mapping objects to the correct values
while leaving the associated response types unchanged.
| def _singularize_resource(self, resource: str) -> str: | ||
| """ | ||
| Convert a plural resource name to its singular form. | ||
| Args: | ||
| resource: Plural resource name (e.g., 'users', 'user_identities', 'identities') | ||
| Returns: | ||
| Singular form of the resource name (e.g., 'user', 'user_identity', 'identity') | ||
| """ | ||
| # Handle special cases | ||
| if resource == 'business': | ||
| return 'business' # Don't remove 's' from 'business' | ||
| elif resource == 'user_identities': | ||
| return 'user_identity' # Replace 'identities' with 'identity' | ||
| elif resource == 'identities': | ||
| return 'identity' # Replace 'identities' with 'identity' | ||
| # Default: remove trailing 's' if present | ||
| elif resource.endswith('s'): | ||
| return resource[:-1] | ||
| else: | ||
| return resource |
There was a problem hiding this comment.
Fix singularization for words ending in 'ies'.
The singularization logic doesn't handle words ending in 'ies' correctly. For example, 'properties' becomes 'propertie' instead of 'property', resulting in methods like create_propertie, update_propertie, delete_propertie.
Apply this diff to fix the singularization:
def _singularize_resource(self, resource: str) -> str:
"""
Convert a plural resource name to its singular form.
Args:
resource: Plural resource name (e.g., 'users', 'user_identities', 'identities')
Returns:
Singular form of the resource name (e.g., 'user', 'user_identity', 'identity')
"""
# Handle special cases
if resource == 'business':
return 'business' # Don't remove 's' from 'business'
elif resource == 'user_identities':
return 'user_identity' # Replace 'identities' with 'identity'
elif resource == 'identities':
return 'identity' # Replace 'identities' with 'identity'
+ # Handle words ending in 'ies' (e.g., 'properties' -> 'property')
+ elif resource.endswith('ies'):
+ return resource[:-3] + 'y'
# Default: remove trailing 's' if present
elif resource.endswith('s'):
return resource[:-1]
else:
return resource📝 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.
| def _singularize_resource(self, resource: str) -> str: | |
| """ | |
| Convert a plural resource name to its singular form. | |
| Args: | |
| resource: Plural resource name (e.g., 'users', 'user_identities', 'identities') | |
| Returns: | |
| Singular form of the resource name (e.g., 'user', 'user_identity', 'identity') | |
| """ | |
| # Handle special cases | |
| if resource == 'business': | |
| return 'business' # Don't remove 's' from 'business' | |
| elif resource == 'user_identities': | |
| return 'user_identity' # Replace 'identities' with 'identity' | |
| elif resource == 'identities': | |
| return 'identity' # Replace 'identities' with 'identity' | |
| # Default: remove trailing 's' if present | |
| elif resource.endswith('s'): | |
| return resource[:-1] | |
| else: | |
| return resource | |
| def _singularize_resource(self, resource: str) -> str: | |
| """ | |
| Convert a plural resource name to its singular form. | |
| Args: | |
| resource: Plural resource name (e.g., 'users', 'user_identities', 'identities') | |
| Returns: | |
| Singular form of the resource name (e.g., 'user', 'user_identity', 'identity') | |
| """ | |
| # Handle special cases | |
| if resource == 'business': | |
| return 'business' # Don't remove 's' from 'business' | |
| elif resource == 'user_identities': | |
| return 'user_identity' # Replace 'identities' with 'identity' | |
| elif resource == 'identities': | |
| return 'identity' # Replace 'identities' with 'identity' | |
| # Handle words ending in 'ies' (e.g., 'properties' -> 'property') | |
| elif resource.endswith('ies'): | |
| return resource[:-3] + 'y' | |
| # Default: remove trailing 's' if present | |
| elif resource.endswith('s'): | |
| return resource[:-1] | |
| else: | |
| return resource |
🤖 Prompt for AI Agents
In kinde_sdk/management/management_client.py around lines 389 to 410, the
singularization routine mishandles words ending in "ies" (e.g., "properties" ->
"propertie"); update the function to first handle the special-case mappings
(business, user_identities, identities), then add a rule that if resource ends
with "ies" replace the "ies" with "y" to produce the correct singular
(properties -> property), and finally fall back to removing a trailing "s" if
present; keep the special-case checks before the generic rules to avoid
regressions.
- Fix singularization for words ending in 'ies' (e.g., 'properties' -> 'property') This fixes the critical issue where 'properties' was incorrectly singularized to 'propertie' instead of 'property', causing incorrect method names. - Fix HTTP status codes to follow REST conventions: - Users create: 200 -> 201 (POST should return 201 Created) - Roles update: 201 -> 200 (PATCH should return 200 OK) These changes address the CodeRabbit review comments on PR kinde-oss#144.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pyproject.tomlis excluded by!**/*.toml
📒 Files selected for processing (1)
requirements.txt(1 hunks)
🔇 Additional comments (2)
requirements.txt (2)
23-27: Conditional pytest dependencies are well-formed and align with PR objectives.The dual-line approach with environment markers is the correct mechanism for conditional dependencies in requirements.txt. Both lines will coexist, and pip will resolve the appropriate version at install time based on the active Python version. The version constraints (pytest<9 for Python 3.9, pytest>=9 for Python 3.10+) directly address the pytest v9 Python 3.10+ requirement and maintain backward compatibility.
32-32: > Likely an incorrect or invalid review comment.
| # pytest: v9 requires Python 3.10+, so we use conditional dependencies | ||
| # Python 3.9: pytest 7.4.0 to <9.0.0 | ||
| # Python 3.10+: pytest 9.0.0+ | ||
| pytest>=7.4.0,<9.0.0; python_version < "3.10" | ||
| pytest>=9.0.0; python_version >= "3.10" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
pytest-asyncio 0.26.0 compatibility pytest 7.4 pytest 9.0 requirements
💡 Result:
Short answer: pytest-asyncio 0.26.0 (released 2025‑03‑25) declares a dependency on pytest but does not pin an upper pytest version; it is known to work with pytest 7.x (e.g. 7.4) and there’s no explicit upper bound in the 0.26.0 release metadata — however pytest 9.0 is a major pytest release and may introduce breaking changes that could require a newer pytest-asyncio. See the pytest-asyncio 0.26.0 release notes and PyPI/GitHub listings. [1][2][3]
Sources:
- pytest-asyncio changelog (0.26.0 release notes). [1]
- pytest-asyncio PyPI project page (package dependency = pytest, no strict upper bound shown). [2]
- pytest changelog / 9.0 notes (major release). [3]
If you want, I can:
- check the exact Requires‑Dist entry from the 0.26.0 wheel/METADATA for a precise version specifier, or
- run a quick compatibility test matrix (pytest-asyncio 0.26.0 vs pytest 7.4 and pytest 9.0) and report failures.
🏁 Script executed:
# Search for lock files and existing pytest-asyncio constraints in the repository
find . -type f \( -name "*.lock" -o -name "pyproject.toml" -o -name "poetry.lock" -o -name "Pipfile.lock" -o -name "*requirements*.txt" \) | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 196
🏁 Script executed:
# Check the requirements.txt file for context around pytest-asyncio and black versions
cat -n requirements.txt | sed -n '20,35p'Repository: kinde-oss/kinde-python-sdk
Length of output: 196
🌐 Web query:
pytest-asyncio 0.26.0 PyPI metadata Requires-Dist pytest version constraint
💡 Result:
Do you want me to look up the exact Requires-Dist pytest constraint from the PyPI metadata for pytest-asyncio 0.26.0?
🌐 Web query:
pytest 9.0 release notes breaking changes pytest-asyncio compatibility
💡 Result:
Short answer
- Pytest 9.0 introduced several breaking changes (notably: removal of older-deprecated APIs, making PytestRemovedIn9Warning errors by default, dropping Python 3.9, and other CLI/behavior changes). See the pytest 9.0 release/changelog for details. [1][2]
- pytest-asyncio added explicit support for pytest 9 in pytest-asyncio 1.3.0 (release notes: "Support for pytest 9"). If you use an older pytest-asyncio (0.23.x / 0.22.x etc.) you may see breakage; upgrade pytest-asyncio to 1.3.0+ for pytest 9 compatibility, or pin pytest <9.0. [3][4]
What to do
- If you rely on pytest-asyncio: upgrade pytest-asyncio to >=1.3.0 when moving to pytest 9.0. [3]
- If you cannot upgrade pytest-asyncio, keep pytest <9.0 (pin in requirements) until your plugin/supporting library is updated. [4]
Relevant links
- pytest changelog / 9.0 notes. [1]
- pytest 9.0 release notes on GitHub (summary of breaking changes). [2]
- pytest-asyncio changelog (1.3.0 notes: support for pytest 9). [3]
- Issue threads about pytest-asyncio breakages with newer pytest versions (background). [4]
References
[1] pytest changelog — docs.pytest.org. (docs.pytest.org)
[2] pytest 9.0 release (GitHub releases). (github.com)
[3] pytest-asyncio changelog (1.3.0 — "Support for pytest 9"). (pytest-asyncio.readthedocs.io)
[4] GitHub issues showing earlier incompatibilities between pytest and pytest-asyncio. (github.com)
Citations:
- 1: https://docs.pytest.org/en/stable/changelog.html?utm_source=openai
- 2: https://github.com/pytest-dev/pytest/releases?utm_source=openai
- 3: https://pytest-asyncio.readthedocs.io/en/stable/reference/changelog.html?utm_source=openai
- 4: Latest 8.0.0 is breaking on 3.8+ pytest-dev/pytest#11868
Fix pytest-asyncio compatibility with pytest 9.0+.
pytest-asyncio 0.26.0 does not support pytest 9.0+. According to the pytest-asyncio changelog, explicit support for pytest 9 was added only in version 1.3.0 and later. The current configuration will fail on Python 3.10+ environments where pytest 9.0+ is installed.
Use conditional pytest-asyncio versions to match the pytest versions:
- For Python <3.10 (pytest 7.4.x): pytest-asyncio 0.26.0 is compatible
- For Python >=3.10 (pytest 9.0+): upgrade to pytest-asyncio>=1.3.0
Alternatively, upgrade pytest-asyncio to >=1.3.0 unconditionally if it supports Python 3.9.
🤖 Prompt for AI Agents
In requirements.txt around lines 23 to 27, pytest-asyncio is incompatible with
pytest 9.x used on Python >=3.10; update the file to add conditional
pytest-asyncio pins that match the pytest lines: keep pytest-asyncio==0.26.0 for
python_version < "3.10" and require pytest-asyncio>=1.3.0 for python_version >=
"3.10" (or alternatively bump pytest-asyncio to >=1.3.0 unconditionally if that
version supports Python 3.9), ensuring the conditional markers align with the
existing pytest lines so CI on Python 3.10+ uses a pytest-asyncio that supports
pytest 9.0+.
…atibility Address CodeRabbit review comment: pytest-asyncio 0.26.0 does not support pytest 9.0+. According to pytest-asyncio changelog, explicit support for pytest 9 was added in version 1.3.0+. Changes: - Add conditional pytest-asyncio dependencies matching pytest versions: - Python <3.10: pytest-asyncio>=0.26.0 (compatible with pytest 7.4.x) - Python >=3.10: pytest-asyncio>=1.3.0 (required for pytest 9.0+) - Update both requirements.txt and pyproject.toml - Add comments explaining the version requirements This ensures Python 3.10+ environments with pytest 9.0+ use a compatible pytest-asyncio version, preventing runtime failures. Fixes: CodeRabbit review comment on PR kinde-oss#144
2 participants
Explain your changes
Implement conditional pytest dependencies to maintain backward compatibility
with Python 3.9 while allowing Python 3.10+ to use pytest v9.
Changes:
This resolves the Python 3.9 compatibility issue in PR #134 where pytest v9
requires Python 3.10+. The conditional dependencies ensure:
Fixes: #134"
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.