- Notifications
You must be signed in to change notification settings - Fork 66
feat: Python SDK update for version 13.6.1 #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis patch release (13.6.1) systematically refactors parameter handling across all Appwrite SDK service modules. The changes replace unconditional assignments of optional API parameters with conditional checks that only include parameters when their values are not None. Version strings are updated in CHANGELOG.md, setup.py, and client.py. One public method signature in sites.py is modified to add an Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
appwrite/services/tokens.py (1)
91-91: Make optional expire conditional to avoid sending null valuesBoth create_file_token and update include expire even when None. Align with PR pattern by guarding it.
Apply this diff:
@@ - api_params['expire'] = expire + if expire is not None: + api_params['expire'] = expire @@ - api_params['expire'] = expire + if expire is not None: + api_params['expire'] = expireAlso applies to: 157-157
appwrite/services/databases.py (2)
1655-1658: Also guard Optional default in geometry attribute create endpointscreate_line_attribute, create_point_attribute, and create_polygon_attribute set default unconditionally; make it conditional.
Apply this pattern:
@@ def create_line_attribute(...): - api_params['key'] = key - api_params['required'] = required - api_params['default'] = default + api_params['key'] = key + api_params['required'] = required + if default is not None: + api_params['default'] = defaultMirror for create_point_attribute and create_polygon_attribute.
Also applies to: 1771-1774, 1887-1890
1655-1658: Fix unguarded optional assignments across multiple methods in databases.py, tables_db.py, and tokens.pyThe verification confirms extensive unguarded assignments of optional parameters throughout the codebase:
- databases.py (lines 1657, 819, and 14+ additional locations): Unguarded
api_params['default'] = defaultwithout None checks- tables_db.py (12+ locations): Same unguarded
defaultpattern- tokens.py (lines 91, 157): Unguarded
api_params['expire'] = expireassignmentsTo maintain consistency with guarded patterns elsewhere (e.g., line 759:
if default is not None:), wrap all these assignments with appropriate None checks before assignment.appwrite/services/tables_db.py (1)
1576-1578: Inconsistent parameter handling for geometric columndefaultvalues.The
defaultparameter increate_line_columnis optional with a default value ofNone, but it's being unconditionally assigned toapi_paramson Line 1578. This is inconsistent with the conditional parameter handling pattern applied throughout this PR.The same issue exists in:
update_line_column(Line 1633)create_point_column(Line 1688)update_point_column(Line 1743)create_polygon_column(Line 1798)update_polygon_column(Line 1853)Apply this pattern to these geometric column methods:
api_params['key'] = key api_params['required'] = required -api_params['default'] = default +if default is not None: + api_params['default'] = default
🧹 Nitpick comments (1)
appwrite/services/teams.py (1)
283-294: Validate that at least one of email, userId, or phone is providedServer prioritizes userId > email > phone, but when none are supplied the request will fail downstream. Add a local guard.
Apply this diff:
@@ if roles is None: raise AppwriteException('Missing required parameter: "roles"') api_path = api_path.replace('{teamId}', team_id) + # Ensure at least one identifier is provided + if not (email or user_id or phone): + raise AppwriteException('Provide at least one of "user_id", "email", or "phone"') + if email is not None: api_params['email'] = email if user_id is not None: api_params['userId'] = user_id if phone is not None: api_params['phone'] = phone
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
CHANGELOG.md(1 hunks)appwrite/client.py(1 hunks)appwrite/services/account.py(7 hunks)appwrite/services/avatars.py(6 hunks)appwrite/services/databases.py(48 hunks)appwrite/services/functions.py(13 hunks)appwrite/services/health.py(14 hunks)appwrite/services/messaging.py(39 hunks)appwrite/services/sites.py(11 hunks)appwrite/services/storage.py(9 hunks)appwrite/services/tables_db.py(48 hunks)appwrite/services/teams.py(4 hunks)appwrite/services/tokens.py(1 hunks)appwrite/services/users.py(18 hunks)setup.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
appwrite/services/functions.py (1)
appwrite/query.py (1)
search(71-72)
🔇 Additional comments (13)
appwrite/services/avatars.py (1)
49-54: LGTM: conditional parameter inclusion is consistent and correctGuards correctly omit None for all optional fields; matches PR objective.
Also applies to: 95-101, 174-179, 219-222, 261-268, 307-312
appwrite/services/teams.py (1)
38-43: LGTM: optional fields now added conditionallylist, list_memberships, and create follow the new pattern and look good.
Also applies to: 224-229, 83-84
appwrite/services/account.py (1)
76-77: LGTM: conditional param handling is clean and consistentOptional fields are correctly omitted when None.
Also applies to: 148-151, 235-238, 594-595, 1156-1157, 1204-1206, 1249-1254
appwrite/services/health.py (1)
98-99: LGTM: health endpoints now skip None-valued optionsConsistent guards across endpoints; no issues found.
Also applies to: 169-170, 198-199, 229-233, 260-261, 296-297, 325-326, 354-355, 383-384, 412-413, 441-442, 470-471, 499-500, 528-529
appwrite/services/users.py (1)
41-46: LGTM: optional fields are conditionally included; requireds remain enforcedPattern is consistent across list/create/update flows.
Also applies to: 86-93, 222-228, 547-551, 688-691, 768-772, 809-815, 1412-1416, 1554-1559, 1635-1638
appwrite/services/storage.py (1)
42-47: LGTM! Conditional parameter handling implemented correctly.The refactoring consistently applies None checks before including optional parameters in API payloads across all methods. This prevents sending null values to the API and aligns with the broader SDK cleanup initiative.
Also applies to: 101-116, 202-217, 288-293, 347-349, 435-438, 517-518, 580-603, 643-644
appwrite/services/messaging.py (1)
40-45: LGTM! Comprehensive conditional parameter refactoring.The changes systematically apply None checks across all messaging-related methods, including email, push, SMS, and all provider configurations (APNS, FCM, Mailgun, Sendgrid, etc.). The implementation is consistent and prevents unnecessary null values in API requests.
Also applies to: 107-124, 180-201, 270-305, 375-410, 459-468, 514-525, 626-629, 665-668, 700-705, 755-766, 813-826, 869-872, 911-916, 971-986, 1037-1054, 1101-1108, 1151-1160, 1211-1222, 1269-1282, 1333-1344, 1391-1404, 1471-1492, 1551-1576, 1623-1630, 1673-1682, 1729-1736, 1779-1788, 1835-1842, 1885-1894, 1941-1948, 1991-2000, 2101-2104, 2140-2143, 2175-2180, 2220-2221, 2291-2294, 2363-2366, 2404-2409
appwrite/services/sites.py (1)
44-49: LGTM! Conditional parameter handling and upload progress support.The refactoring consistently applies None checks across all methods. Additionally, Line 450 enhances the
create_deploymentmethod signature with anon_progressparameter, enabling upload progress callbacks for better user experience during deployment uploads.Also applies to: 126-154, 302-331, 440-445, 495-502, 606-607, 656-657, 773-774, 848-851, 1003-1004, 1090-1093
appwrite/services/functions.py (1)
43-48: LGTM! Conditional parameter handling implemented correctly.All other methods in this file correctly implement the conditional parameter pattern, only including optional parameters in API payloads when they are not None.
Also applies to: 122-151, 434-439, 491-496, 542-543, 604-605, 654-655, 771-772, 846-849, 893-904, 1057-1058, 1144-1147
appwrite/services/tables_db.py (1)
41-46: LGTM: Consistent conditional parameter handling.The systematic refactoring to conditionally include optional parameters only when they are not
Noneimproves API payload cleanliness and aligns with the PR objective. The pattern is applied consistently across all methods (except for the geometric column methods flagged separately).Also applies to: 87-88, 117-118, 146-147, 215-218, 285-286, 358-359, 430-435, 485-490, 578-583, 664-667, 722-725, 837-840, 953-956, 1075-1078, 1202-1209, 1464-1467, 1915-1922, 1986-1991, 2109-2112, 2307-2310, 2353-2356, 2417-2420, 2552-2557, 2611-2614, 2661-2662, 2710-2711, 2756-2761, 2804-2807, 2856-2859, 2909-2914, 2965-2970, 3017-3018, 3075-3080, 3137-3142
CHANGELOG.md (1)
3-5: LGTM: Changelog entry accurately reflects the changes.The changelog entry correctly documents the fix for handling
Nonevalues in nullable parameters, which aligns with the systematic refactoring applied across the SDK service modules.appwrite/client.py (1)
18-18: LGTM: Version strings correctly updated.The SDK version strings in the HTTP headers are properly updated to 13.6.1, consistent with the version bump in setup.py and the changelog entry.
Also applies to: 22-22
setup.py (1)
11-11: LGTM: Package version correctly updated.The version and download URL are properly updated to 13.6.1, consistent with the SDK version bump across all files.
Also applies to: 21-21
| api_params['required'] = required | ||
| api_params['default'] = default | ||
| api_params['newKey'] = new_key | ||
| if new_key is not None: | ||
| api_params['newKey'] = new_key | ||
| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard Optional default in attribute update endpoints
update_*_attribute methods still send default even when None. This deviates from the PR’s “omit None” policy and risks overwriting defaults with null or triggering API validation errors.
Apply this pattern (example for boolean/email shown; replicate for others):
@@ def update_boolean_attribute(...): - api_params['required'] = required - api_params['default'] = default + api_params['required'] = required + if default is not None: + api_params['default'] = default @@ def update_email_attribute(...): - api_params['required'] = required - api_params['default'] = default + api_params['required'] = required + if default is not None: + api_params['default'] = defaultRepeat the same guard for update_datetime_attribute, update_float_attribute, update_integer_attribute, update_ip_attribute, update_line_attribute, update_point_attribute, update_polygon_attribute, update_string_attribute, and update_url_attribute.
Also applies to: 939-943, 1062-1066, 1332-1339, 1471-1478, 1598-1602, 1714-1718, 1830-1834, 1946-1950, 2151-2157, 2276-2280
🤖 Prompt for AI Agents
In appwrite/services/databases.py around lines 818-822 (and also at 939-943, 1062-1066, 1332-1339, 1471-1478, 1598-1602, 1714-1718, 1830-1834, 1946-1950, 2151-2157, 2276-2280) the update_*_attribute methods unconditionally set api_params['default'] = default which will send null to the API; change each occurrence to only add the 'default' key when default is not None (i.e., wrap assignment in a guard like: if default is not None: api_params['default'] = default) and apply the same pattern used for newKey/new_key so no None values are sent to the API. | api_params['scopes'] = scopes | ||
| if installation_id is not None: | ||
| api_params['installationId'] = installation_id | ||
| api_params['providerRepositoryId'] = provider_repository_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent parameter handling for provider_repository_id.
This line unconditionally assigns provider_repository_id to api_params, which contradicts the PR's objective to conditionally include only non-None optional parameters. All surrounding parameters (lines 295-316, 318-325) use conditional checks.
Apply this diff to align with the refactoring pattern:
- api_params['providerRepositoryId'] = provider_repository_id + if provider_repository_id is not None: + api_params['providerRepositoryId'] = provider_repository_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.
| api_params['providerRepositoryId'] = provider_repository_id | |
| if provider_repository_id is not None: | |
| api_params['providerRepositoryId'] = provider_repository_id |
🤖 Prompt for AI Agents
In appwrite/services/functions.py around line 317, the assignment api_params['providerRepositoryId'] = provider_repository_id is unconditional but should follow the refactor pattern used for other optional params; change it to only add providerRepositoryId to api_params when provider_repository_id is not None (or truthy per surrounding checks), matching the conditional checks used for lines 295-316 and 318-325 so optional parameters are only included when provided.
This PR contains updates to the Python SDK for version 13.6.1.
Summary by CodeRabbit
Bug Fixes
Chores