Feature/python sdk#135
Merged
yash-pouranik merged 9 commits intogeturbackend:mainfrom Apr 26, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 24 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughA complete Python SDK for urBackend is introduced, featuring a main client entrypoint with lazy-loaded auth, database, storage, and mail modules. The SDK wraps HTTP communication, manages authentication tokens, supports database CRUD and querying, handles file uploads, and provides server-side email functionality. Comprehensive tests and documentation accompany the implementation. Changes
Sequence DiagramsequenceDiagram
participant User as User Code
participant Client as UrBackendClient
participant HTTP as UrBackendHTTP
participant Auth as AuthModule
participant DB as DatabaseModule
participant Server as urBackend API
User->>Client: UrBackendClient(api_key)
Client->>HTTP: Initialize with api_key, base_url
User->>Client: client.auth
Client->>Auth: Lazy-load AuthModule(http)
User->>Auth: auth.login(email, password)
Auth->>HTTP: POST /api/userAuth/login
HTTP->>Server: request + x-api-key header
Server-->>HTTP: { success: true, data: { accessToken } }
HTTP-->>Auth: accessToken
Auth->>Auth: cache token internally
Auth-->>User: { accessToken, ... }
User->>Client: client.db
Client->>DB: Lazy-load DatabaseModule(http)
User->>DB: db.insert(collection, data)
DB->>HTTP: POST /api/db/{collection}
HTTP->>HTTP: Add Authorization: Bearer <cached_token>
HTTP->>Server: request + headers
Server-->>HTTP: { success: true, data: { _id, ... } }
HTTP-->>DB: document
DB-->>User: created document
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (6)
sdks/urbackend-python/urbackend/client.py (1)
95-102: Duplicatedsk_live_warning logic — extract to a helper.The same secret-key warning is repeated almost verbatim in
__init__(lines 95–102) andconnect(lines 192–197). Extract into a small private function/static method so the wording stays consistent and a future change touches one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/client.py` around lines 95 - 102, Extract the duplicated secret-key warning into a single private helper (e.g., a staticmethod or function named _warn_if_secret_key) and call it from both __init__ and connect instead of repeating the warning block; implement _warn_if_secret_key(api_key, warnings) to check api_key.startswith("sk_live_") and issue the warnings.warn with the existing message and stacklevel=2, then replace the inline warning blocks in __init__ and connect with a call to _warn_if_secret_key(api_key) to keep wording consistent.sdks/urbackend-python/urbackend/http.py (1)
39-39: Endpoint derivation discards query string and host.
urlparse(response.url).pathonly keeps the path. For multi-tenant deployments or debugging, callers may benefit from preserving the query (e.g.,path?count=true) onendpoint. Optional polish.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/http.py` at line 39, The assignment to endpoint currently drops query and host by using urlparse(response.url).path; change it to preserve the query string by parsing once (e.g., parsed = urlparse(response.url)) and set endpoint = parsed.path + ('?' + parsed.query if parsed.query else '') so endpoint includes path and query (keep host out if you want only path+query); update the code around the endpoint variable in urbackend/http.py where urlparse(response.url).path is used.sdks/urbackend-python/urbackend/storage.py (1)
92-97: Whole-file read into memory.
file.read()(andpath_obj.read_bytes()above) load the entire payload into RAM before computingsizeandPUT-ing to the signed URL. Acceptable for a v0.1.0 SDK, but for parity with the TypeScript SDK and to support multi-GB uploads, consider streaming viarequests.put(..., data=open(path, "rb"))and computingsizefromos.path.getsize/fstatinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/storage.py` around lines 92 - 97, The current implementation reads the entire file into memory via path_obj.read_bytes() and file.read() (see the branch handling IOBase / file-like objects), which prevents streaming large uploads; update the logic to stream the payload instead: for path-like inputs use os.path.getsize(path) and open(path, "rb") as the data source passed into requests.put (or the upload method), and for file-like IOBase inputs use os.fstat(file.fileno()).st_size (or seek/tell fallback) to determine size and pass the file object itself as the data parameter without calling read(); keep resolving the filename using getattr(file, "name", None) / os.path.basename as before and ensure you close any files you open.sdks/urbackend-python/urbackend/auth.py (1)
95-96: Document the dual-key response pattern or simplify the fallback logic.The backend intentionally returns both
tokenandaccessTokenkeys with identical values on all auth endpoints (login, signup, refresh). The fallback pattern in lines 95 and 181 (response.get("accessToken") or response.get("token")) is defensive but unnecessary. Either document why both keys are provided (for backwards compatibility), or simplify toresponse.get("accessToken")since both keys are always present. The current approach masks the actual contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/auth.py` around lines 95 - 96, The code sets self._session_token using a defensive fallback (response.get("accessToken") or response.get("token")); update the implementation to reflect the backend contract by using only response.get("accessToken") (remove the redundant fallback) and add a short docstring or inline comment in the auth methods (e.g., the login/signup/refresh handlers where self._session_token is set and the occurrence at the other location currently using the same pattern) explaining that the backend returns both keys for backward compatibility but accessToken is the canonical key; ensure both locations are updated to use accessToken-only and include the brief comment referencing the dual-key backward-compatibility behavior.sdks/urbackend-python/urbackend/db.py (2)
17-48: Nit:filtershadows the Python builtin (Ruff A002).The
filterparameter shadows the built-infilter()across_build_params,get_all(Line 167), andcount(Line 234). Consider renaming tofilter_(orquery) to silence Ruff A002 and avoid surprises iffilter()is ever needed inside the function body.Also note the merge order in
_build_params:params.update(filter)runs first, then named keys (sort,limit, …) are set, and finallyparams.update(extra). This means a key likesortinsidefilteris silently overridden by thesort=argument, while**extrasilently overrides everything. Worth documenting (or guarding against collisions) so callers are not surprised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/db.py` around lines 17 - 48, Rename the shadowing parameter named filter in _build_params and the related functions get_all and count to filter_ (or query) to avoid clobbering the built-in filter(); update all callsites accordingly. While there, make the merge order explicit: move params.update(extra) earlier (or validate/raise on key collisions) so **extra does not silently overwrite named keys, and consider merging the incoming filter_ after setting named keys or add a brief docstring note in _build_params explaining that named args override filter_ but extra overrides everything if you keep the current order. Ensure you update the function signatures for _build_params, get_all, and count and their internal references to use the new name.
66-102:CollectionRef.findis missingskipandexpand, diverging fromget_all.
DatabaseModule.get_allacceptsskipandexpand, but the chainablefind()alias does not forward them. Users of theclient.db.collection(...).find(...)style (the documented preferred API) cannot paginate viaskipor expand fields without dropping back toget_all. Consider adding both parameters and forwarding them toself._db.get_all(...)for parity.♻️ Proposed change
def find( self, query: Optional[Dict[str, Any]] = None, *, sort: Optional[str] = None, limit: Optional[int] = None, page: Optional[int] = None, + skip: Optional[int] = None, populate: Optional[Union[str, List[str]]] = None, + expand: Optional[Union[str, List[str]]] = None, token: Optional[str] = None, ) -> List[Dict[str, Any]]: ... return self._db.get_all( self._name, filter=query, sort=sort, limit=limit, page=page, + skip=skip, populate=populate, + expand=expand, token=token, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/db.py` around lines 66 - 102, CollectionRef.find currently omits the skip and expand parameters that DatabaseModule.get_all supports; update the signature of CollectionRef.find to accept skip: Optional[int] = None and expand: Optional[Union[str, List[str]]] = None and forward them into the call to self._db.get_all(self._name, ...) so that skip=skip and expand=expand are passed through alongside filter=query, sort, limit, page, populate, and token to maintain parity with DatabaseModule.get_all.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdks/urbackend-python/pyproject.toml`:
- Line 10: The pyproject.toml currently uses license = "MIT" which requires
setuptools ≥77 but the project allows setuptools>=68; update the file so builds
don't break by either (A) bumping the build requirement in the tool.requires (or
requires) entry to "setuptools>=77" to keep the simple license string, or (B)
switch to the legacy license table and classifier by replacing license = "MIT"
with license = { text = "MIT" } and add the classifier "License :: OSI Approved
:: MIT License" to the project's classifiers; pick one approach and apply it
consistently in pyproject.toml.
In `@sdks/urbackend-python/README.md`:
- Line 235: Update the API table entry for the social_exchange function to match
the actual signature used in examples and code: change the second parameter name
from provider to token so the signature reads social_exchange(rt_code, token)
and the return type remains dict; ensure this matches the example usage that
calls client.auth.social_exchange(rt_code, token) and any other references to
social_exchange in the README.
In `@sdks/urbackend-python/test_sdk.py`:
- Around line 196-365: The section numbering is out of order because the Logout
block (the separator("16. Logout") and subsequent client.auth.logout() /
client.close() calls inside main()) appears after sections 17–23; move the
entire Logout block (the separator call labeled "16. Logout", the logout call
result = client.auth.logout(), the prints that follow, and the subsequent
client.close()) so it appears immediately before the "Enhanced Tests" header
(i.e., before the block that prints "END OF ENHANCED TESTS" or before section
17), or alternatively renumber that separator and its label to match its current
position (e.g., change "16. Logout" to "24. Logout") by updating the
separator("16. Logout") string and any related comments to keep section ordering
consistent; locate the block by searching for separator("16. Logout") and the
calls to client.auth.logout() and client.close().
- Line 60: The print statement uses an unnecessary f-string modifier causing
Ruff F541; change the call print(f"✅ Login successful!") to a plain string
print("✅ Login successful!") (locate the print in test_sdk.py — the login
success message) so the string has no f-prefix and the static analysis warning
is resolved.
- Around line 368-371: There is a stray trailing triple-quoted string after the
call to main() that serves as dead/confusing code; remove the dangling docstring
(the triple-quoted """Description": "Live integration test script...""") that
appears after main() or, if you intended it as module documentation, move it to
the top of the file as the module docstring; ensure only one proper module
docstring exists and that the main() call remains the last executable statement
in the file.
- Around line 28-32: Replace the hardcoded placeholder constants API_KEY,
BASE_URL, EMAIL, PASSWORD, and COLLECTION with values read from environment
variables (e.g., os.environ or similar) and raise a clear, early error when any
required env var is missing; update any code that references these variables
(API_KEY, BASE_URL, EMAIL, PASSWORD, COLLECTION) to use the env-derived values
so the script fails fast with a descriptive message rather than attempting
requests with invalid placeholders.
In `@sdks/urbackend-python/urbackend/auth.py`:
- Around line 375-392: The social_start_url method currently embeds
self._http._api_key into a browser redirect which may leak secret keys; modify
social_start_url to detect if self._http._api_key starts with "sk_live_" (or any
secret key prefix your service uses) and raise a clear exception (e.g.,
ValueError) explaining that only publishable keys are allowed for browser
redirects, or alternatively return a URL that omits the key and requires
server-side exchange; also update the docstring of social_start_url to state
this constraint. Ensure the check references social_start_url and
self._http._api_key (and the client constructor behavior if needed) so callers
cannot accidentally pass a secret key into a browser redirect.
- Around line 390-392: The method social_start_url is reaching into
UrBackendHTTP's private attributes (_base_url and _api_key); add read-only
properties base_url and api_key to the UrBackendHTTP class and update
social_start_url to use self._http.base_url and self._http.api_key instead of
the private names so callers don't break if internal storage changes; ensure the
new properties return the same values and are documented as read-only accessors.
- Around line 143-151: The silent broad except in the logout flow (around
self._http.request in the logout method) should be replaced: catch only the
HTTP/SDK-specific exception(s) thrown by your HTTP client (e.g.,
httpx.HTTPError, urllib3.exceptions.HTTPError or your SDK's specific error
class) and emit a warning or logger.warn with the exception details instead of
swallowing it; allow other unexpected exceptions to propagate (or re-raise them)
so real bugs aren't hidden. Ensure you still set self._session_token = None and
return result as before, but only suppress/log the known SDK/http errors
surrounding the call to self._http.request("/api/userAuth/logout").
In `@sdks/urbackend-python/urbackend/client.py`:
- Around line 199-209: connect() recreates self._http but does not forward
caller-provided extra_headers causing them to be lost; persist the headers on
the instance or accept an extra_headers parameter to connect and pass them to
the new UrBackendHTTP. Update the class __init__ to store the incoming
extra_headers on self (e.g., self._extra_headers) and then modify connect() to
construct UrBackendHTTP(api_key=api_key, base_url=base_url,
extra_headers=self._extra_headers) (or add an extra_headers argument to
connect() and use that value), ensuring any lazy modules (self._auth, self._db,
self._storage, self._mail) are still reset and self._http is replaced with the
new UrBackendHTTP instance.
In `@sdks/urbackend-python/urbackend/db.py`:
- Around line 419-425: The delete() method currently inspects the DELETE
response body to decide deleted:true/false even though _http.request already
raises on non-2xx (UrBackendError), which causes false negatives; change
delete() (the call to self._http.request in delete()) to treat the absence of an
exception as success: remove/stop relying on result.get("id") or message checks
and simply return {"deleted": True} after the request completes (still return
{"deleted": False} only if an exception is raised), ensuring 204/empty bodies or
different field names (e.g., "_id") don't produce incorrect false results.
- Around line 230-256: The count method currently lets NotFoundError propagate
while get_all swallows it and returns empty results; update db.count (the count
function) to mirror get_all's behavior by catching NotFoundError around the call
to self._http.request(path, params=params, token=token) and returning 0 when a
NotFoundError is raised, while allowing other exceptions to propagate unchanged;
reference the count function, get_all behavior, the NotFoundError exception, and
the self._http.request call when making the change.
In `@sdks/urbackend-python/urbackend/http.py`:
- Around line 199-202: The current unwrap logic unconditionally returns
payload["data"] whenever a top-level "data" key exists; tighten this in the
response handling code in http.py by only unwrapping when the envelope shape
matches the expected API contract (e.g., payload is a dict AND contains a
"success" key that is a boolean and a "data" key) so you don't accidentally
unwrap arbitrary bodies; update the condition that references payload to check
for "success" (isinstance(payload.get("success"), bool)) as well as "data"
before returning payload["data"] so non-envelope responses or error-shaped
bodies are left intact.
- Around line 61-64: The Retry-After header parsing in _parse_api_error
currently does int(retry_after_raw) which will raise ValueError for HTTP-date
formats and mask the intended RateLimitError; change the logic in
_parse_api_error where it reads response.headers.get("Retry-After") (variable
retry_after_raw) to try parsing as an integer first, and if that fails fallback
to parsing an HTTP-date (e.g., using email.utils.parsedate_to_datetime or
equivalent) to compute delta-seconds, then set retry_after to the seconds value
or None; ensure any parsing exceptions are caught so _parse_api_error always
returns a RateLimitError(instance) rather than letting a ValueError propagate.
In `@sdks/urbackend-python/urbackend/mail.py`:
- Around line 68-88: The docstring for the mail sending API is inconsistent: the
Returns section documents an "id" key while the example accesses "messageId";
update the docstring for the send method (referenced as client.mail.send / send)
so the return-key name is consistent—either change the Returns entry to
"messageId" (and update the description of that field) or update the example to
use result.get("id"), and ensure any other referenced keys (provider,
monthlyUsage, monthlyLimit) match the actual return payload names used by the
send implementation.
---
Nitpick comments:
In `@sdks/urbackend-python/urbackend/auth.py`:
- Around line 95-96: The code sets self._session_token using a defensive
fallback (response.get("accessToken") or response.get("token")); update the
implementation to reflect the backend contract by using only
response.get("accessToken") (remove the redundant fallback) and add a short
docstring or inline comment in the auth methods (e.g., the login/signup/refresh
handlers where self._session_token is set and the occurrence at the other
location currently using the same pattern) explaining that the backend returns
both keys for backward compatibility but accessToken is the canonical key;
ensure both locations are updated to use accessToken-only and include the brief
comment referencing the dual-key backward-compatibility behavior.
In `@sdks/urbackend-python/urbackend/client.py`:
- Around line 95-102: Extract the duplicated secret-key warning into a single
private helper (e.g., a staticmethod or function named _warn_if_secret_key) and
call it from both __init__ and connect instead of repeating the warning block;
implement _warn_if_secret_key(api_key, warnings) to check
api_key.startswith("sk_live_") and issue the warnings.warn with the existing
message and stacklevel=2, then replace the inline warning blocks in __init__ and
connect with a call to _warn_if_secret_key(api_key) to keep wording consistent.
In `@sdks/urbackend-python/urbackend/db.py`:
- Around line 17-48: Rename the shadowing parameter named filter in
_build_params and the related functions get_all and count to filter_ (or query)
to avoid clobbering the built-in filter(); update all callsites accordingly.
While there, make the merge order explicit: move params.update(extra) earlier
(or validate/raise on key collisions) so **extra does not silently overwrite
named keys, and consider merging the incoming filter_ after setting named keys
or add a brief docstring note in _build_params explaining that named args
override filter_ but extra overrides everything if you keep the current order.
Ensure you update the function signatures for _build_params, get_all, and count
and their internal references to use the new name.
- Around line 66-102: CollectionRef.find currently omits the skip and expand
parameters that DatabaseModule.get_all supports; update the signature of
CollectionRef.find to accept skip: Optional[int] = None and expand:
Optional[Union[str, List[str]]] = None and forward them into the call to
self._db.get_all(self._name, ...) so that skip=skip and expand=expand are passed
through alongside filter=query, sort, limit, page, populate, and token to
maintain parity with DatabaseModule.get_all.
In `@sdks/urbackend-python/urbackend/http.py`:
- Line 39: The assignment to endpoint currently drops query and host by using
urlparse(response.url).path; change it to preserve the query string by parsing
once (e.g., parsed = urlparse(response.url)) and set endpoint = parsed.path +
('?' + parsed.query if parsed.query else '') so endpoint includes path and query
(keep host out if you want only path+query); update the code around the endpoint
variable in urbackend/http.py where urlparse(response.url).path is used.
In `@sdks/urbackend-python/urbackend/storage.py`:
- Around line 92-97: The current implementation reads the entire file into
memory via path_obj.read_bytes() and file.read() (see the branch handling IOBase
/ file-like objects), which prevents streaming large uploads; update the logic
to stream the payload instead: for path-like inputs use os.path.getsize(path)
and open(path, "rb") as the data source passed into requests.put (or the upload
method), and for file-like IOBase inputs use os.fstat(file.fileno()).st_size (or
seek/tell fallback) to determine size and pass the file object itself as the
data parameter without calling read(); keep resolving the filename using
getattr(file, "name", None) / os.path.basename as before and ensure you close
any files you open.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c9f0f34-fed8-4d13-8c87-2f5c575e67e3
📒 Files selected for processing (17)
sdks/urbackend-python/.gitignoresdks/urbackend-python/README.mdsdks/urbackend-python/pyproject.tomlsdks/urbackend-python/test_sdk.pysdks/urbackend-python/tests/__init__.pysdks/urbackend-python/tests/test_auth.pysdks/urbackend-python/tests/test_db.pysdks/urbackend-python/tests/test_exceptions.pysdks/urbackend-python/tests/test_http.pysdks/urbackend-python/urbackend/__init__.pysdks/urbackend-python/urbackend/auth.pysdks/urbackend-python/urbackend/client.pysdks/urbackend-python/urbackend/db.pysdks/urbackend-python/urbackend/exceptions.pysdks/urbackend-python/urbackend/http.pysdks/urbackend-python/urbackend/mail.pysdks/urbackend-python/urbackend/storage.py
Collaborator
|
@Navyasree-ulava |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
sdks/urbackend-python/urbackend/client.py (1)
169-212: LGTM onconnect().Good fix for the previously-flagged
extra_headersregression — it now persists/forwards them, and the explicitextra_headersparameter follows the documented override semantics. Lazy module reset ensures subsequent property access binds to the newUrBackendHTTPsession.One nit (optional): the
api_keyvalidation +sk_live_warning logic is duplicated between__init__andconnect(); extracting it into a private_validate_api_key(api_key)helper would keep the two entry points in sync if the rules ever evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/client.py` around lines 169 - 212, Extract the duplicated api key checks into a private helper (e.g., _validate_api_key(self, api_key)) that raises ValueError for empty keys and emits the sk_live_ warning, then call this helper from both __init__ and connect instead of duplicating the logic; update references to the current inline checks in connect (and __init__) to use _validate_api_key(api_key) so validation/warning rules remain centralized and consistent.sdks/urbackend-python/urbackend/db.py (1)
17-48:filterkeys can silently overwrite reserved query parameters.
params.update(filter)at line 32 is applied first, thensort/limit/page/skip/populate/expand/countoverwrite anything with the same name. This is fine in the typical case, but if a user callsget_all("posts", filter={"limit": "small"})(a real document field namedlimit), the explicitlimit=10argument silently clobbers their filter — and conversely,**extraat line 47 can silently clobbersort/limit. Consider either prefixing filter keys (e.g.filter[<key>]=...to mirror common REST conventions on the backend) or at least documenting the reserved param names in the docstring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/urbackend/db.py` around lines 17 - 48, The helper _build_params currently calls params.update(filter) and later merges explicit args and extra, which allows filter keys or **extra to silently overwrite reserved query params like sort, limit, page, skip, populate, expand, count; change _build_params to namespace filter keys (e.g., convert each key k from filter into "filter[k]": value) before merging so document fields named "limit" etc. do not collide, or alternatively validate and raise on conflicts between filter keys/**extra and the reserved names; update the docstring to list reserved params (sort, limit, page, skip, populate, expand, count) and reference the function names _build_params, the filter handling branch that uses params.update(filter), and the extra merge that does params.update(extra) so reviewers can find and adjust the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdks/urbackend-python/README.md`:
- Line 235: The README table row merged `social_exchange(rt_code, token)` and
`set_token(token)` into one line producing 7 cells; split that single table line
into two separate markdown table rows so each row has three cells: one for
`social_exchange(rt_code, token)` with its return type `dict`, and one for
`set_token(token)` with return type `None`, ensuring the pipe separators align
with the other table rows (use the exact function names `social_exchange` and
`set_token` to locate the combined line and separate them into distinct lines).
In `@sdks/urbackend-python/test_sdk.py`:
- Around line 41-42: Remove the stray inline comment "# ← a collection in your
project" that follows the sys.exit(...) call; update the line containing
sys.exit(f"Missing required env vars: {', '.join(missing)}") (referencing the
missing variable and sys.exit call) to have no trailing comment so the error
exit message reads cleanly.
- Around line 17-23: The test imports load_dotenv from python-dotenv in
sdks/urbackend-python/test_sdk.py but that package is missing from the dev
extras; update pyproject.toml to add "python-dotenv" (or
"python-dotenv>=<minimum_version>" if you prefer a version constraint) under the
[project.optional-dependencies].dev (dev extras) so pip install -e ".[dev]"
installs it and the test no longer raises ImportError.
---
Nitpick comments:
In `@sdks/urbackend-python/urbackend/client.py`:
- Around line 169-212: Extract the duplicated api key checks into a private
helper (e.g., _validate_api_key(self, api_key)) that raises ValueError for empty
keys and emits the sk_live_ warning, then call this helper from both __init__
and connect instead of duplicating the logic; update references to the current
inline checks in connect (and __init__) to use _validate_api_key(api_key) so
validation/warning rules remain centralized and consistent.
In `@sdks/urbackend-python/urbackend/db.py`:
- Around line 17-48: The helper _build_params currently calls
params.update(filter) and later merges explicit args and extra, which allows
filter keys or **extra to silently overwrite reserved query params like sort,
limit, page, skip, populate, expand, count; change _build_params to namespace
filter keys (e.g., convert each key k from filter into "filter[k]": value)
before merging so document fields named "limit" etc. do not collide, or
alternatively validate and raise on conflicts between filter keys/**extra and
the reserved names; update the docstring to list reserved params (sort, limit,
page, skip, populate, expand, count) and reference the function names
_build_params, the filter handling branch that uses params.update(filter), and
the extra merge that does params.update(extra) so reviewers can find and adjust
the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53f899fb-1e76-4169-a905-883c00eba7fe
📒 Files selected for processing (8)
sdks/urbackend-python/README.mdsdks/urbackend-python/pyproject.tomlsdks/urbackend-python/test_sdk.pysdks/urbackend-python/urbackend/auth.pysdks/urbackend-python/urbackend/client.pysdks/urbackend-python/urbackend/db.pysdks/urbackend-python/urbackend/http.pysdks/urbackend-python/urbackend/mail.py
✅ Files skipped from review due to trivial changes (1)
- sdks/urbackend-python/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- sdks/urbackend-python/urbackend/mail.py
4 tasks
Collaborator
|
@Navyasree-ulava |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
sdks/urbackend-python/src/mail.py (1)
89-110: Optional: validate that at least one of template/raw-body fields is provided.Currently, calling
send(to=...)with no template_name/template_id and no subject/text/html sends an empty payload and only fails on a server roundtrip withValidationError. A small client-side guard avoids a wasted HTTP call and gives a clearer error.♻️ Proposed guard
payload: Dict[str, Any] = {"to": to} + + if not (template_name or template_id) and not (subject and (text or html)): + raise ValueError( + "send() requires either template_name/template_id, " + "or subject plus text and/or html." + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/mail.py` around lines 89 - 110, Add a client-side guard in the send method to validate that a template or raw body is provided before making the HTTP call: check that at least one of template_name or template_id is set OR at least one of subject, text, or html is set; if none are present, raise a ValueError (or a custom exception) with a clear message and return early instead of calling self._http.request("POST", "/api/mail/send", body=payload). This validation should live in the same function that builds payload (the send method) so it executes before invoking _http.request.sdks/urbackend-python/src/storage.py (1)
125-130: Optional: make the cloud-provider PUT timeout configurable.A hard-coded 120s timeout is fine for small files but will fail legitimate large-file uploads on slow links. Consider exposing it as a parameter on
upload()(e.g.timeout: float = 120) or sourcing it from a module-level default that callers can override.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/storage.py` around lines 125 - 130, The PUT to the cloud uses a hard-coded 120s timeout which can fail large uploads; make the timeout configurable by adding a parameter (e.g., timeout: float = 120) to the upload() function signature and pass that parameter into the requests.put call (which currently creates put_resp). Alternatively, read a module-level default that callers can override and use it when calling requests.put; update any callers/tests of upload() to provide/expect the new parameter if needed.sdks/urbackend-python/src/auth.py (1)
11-14: Nit: consolidate exception imports and group imports cleanly.The two
from .exceptions import …lines (separated byimport warnings) can be merged, and stdlibwarningsbelongs above the relative imports.♻️ Proposed cleanup
-from typing import Any, Dict, Optional - -from .exceptions import AuthError -from .http import UrBackendHTTP -import warnings -from .exceptions import UrBackendError +import warnings +from typing import Any, Dict, Optional + +from .exceptions import AuthError, UrBackendError +from .http import UrBackendHTTP🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/auth.py` around lines 11 - 14, Consolidate and reorder the imports in auth.py: move the stdlib import warnings above the package-relative imports, and merge the two exception imports into a single line importing both AuthError and UrBackendError; keep UrBackendHTTP import alongside the other relative imports so imports are grouped as stdlib, third-party (if any), then local (e.g., warnings first, then from .http import UrBackendHTTP and from .exceptions import AuthError, UrBackendError).sdks/urbackend-python/src/__init__.py (1)
48-56: Optional: sort__all__.Ruff RUF022 flags this. Low priority.
♻️ Proposed sort
__all__ = [ + "AuthError", + "NotFoundError", + "RateLimitError", + "StorageError", "UrBackendClient", "UrBackendError", - "AuthError", - "NotFoundError", - "RateLimitError", - "StorageError", "ValidationError", ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/__init__.py` around lines 48 - 56, The __all__ export list is unsorted which triggers lint RUF022; sort the entries in the __all__ list alphabetically (e.g., AuthError, NotFoundError, RateLimitError, StorageError, UrBackendClient, UrBackendError, ValidationError) so the __all__ symbol contains a consistently ordered list and satisfies the linter.sdks/urbackend-python/src/db.py (2)
17-32: Avoid shadowing thefilterbuiltin (Ruff A002).
filteris used as a kwarg on_build_params,get_all, andcount. Inside these functions it shadows the builtin and trips Ruff A002. Renaming the internal parameter while keeping the public API is straightforward — accept the user-facing keyword via**kwargsor rename towhere/filter_. Up to you which way to go; just make it consistent acrossget_all,count, and_build_params.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/db.py` around lines 17 - 32, The parameter name filter shadows the builtin and should be renamed internally and consistently across _build_params, get_all, and count (e.g., filter_ or where) while preserving the public API; update the function signatures to accept the user-facing keyword but map it to the new internal name (and adjust type hints and docstrings), then update all internal references and any calls to _build_params/get_all/count to pass the renamed variable (or accept **kwargs and pop "filter" into the internal name) so Ruff A002 is resolved without changing external behavior.
80-116:CollectionRef.findis missing parameters thatget_allaccepts.
get_allexposesskipandexpand, butCollectionRef.finddoes not forward them, so the chainableclient.db.collection(name).find(...)API is strictly less expressive than callingclient.db.get_all(...). Consider plumbingskipandexpandthrough to keep the two surfaces in sync.♻️ Proposed fix
def find( self, query: Optional[Dict[str, Any]] = None, *, sort: Optional[str] = None, limit: Optional[int] = None, page: Optional[int] = None, + skip: Optional[int] = None, populate: Optional[Union[str, List[str]]] = None, + expand: Optional[Union[str, List[str]]] = None, token: Optional[str] = None, ) -> List[Dict[str, Any]]: ... return self._db.get_all( self._name, filter=query, sort=sort, limit=limit, page=page, + skip=skip, populate=populate, + expand=expand, token=token, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/db.py` around lines 80 - 116, CollectionRef.find is missing the skip and expand parameters that DatabaseModule.get_all accepts; update the find method signature to accept skip: Optional[int] and expand: Optional[Union[str, List[str]]] (matching get_all's types), add them to the docstring args, and forward them into the call to self._db.get_all (i.e., pass skip=skip and expand=expand alongside filter, sort, limit, page, populate, token) so the collection-level API surface matches DatabaseModule.get_all.sdks/urbackend-python/src/http.py (1)
57-83: Minor:/api/storageerrors with status 401/403/404/429/400 will not surface asStorageError.Because the
"/api/storage" in endpointbranch sits after the status-based branches, every storage failure with a status the upstream branches already handle (auth, missing object 404, validation) is mapped to the generic typed exception, notStorageError. That's defensible (more specific status semantics first), but worth confirming against the TypeScript SDK's contract — if parity calls forStorageErrorto be the umbrella for any storage endpoint failure, swap the order or have storage methods catch and re-wrap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-python/src/http.py` around lines 57 - 83, The storage endpoint check (the if "/api/storage" in endpoint branch) is placed after status-specific branches so 401/403/404/429/400 from storage endpoints never become StorageError; fix by either moving the storage check to run before the status-based returns or by wrapping the specific status-created exceptions into StorageError (e.g., detect "/api/storage" in endpoint at the top of this error-mapping function and return StorageError(message, status, endpoint) for any status, or after creating the specific exception, if endpoint contains "/api/storage" re-wrap/return StorageError with the original exception details so all storage endpoint failures are represented as StorageError).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdks/urbackend-python/README.md`:
- Around line 23-27: Update all README import examples to use the published
package name instead of the repo layout's src module; replace imports like "from
src import UrBackendClient" with "from urbackend import UrBackendClient" (or
whatever the package name in project metadata is) and similarly change "from
myapp.settings import src" to import the actual urbackend instance exported by
your settings module (the instance name declared around line 293). Verify the
package name in pyproject.toml/project metadata and apply that exact top-level
import throughout the README so examples work after pip install.
In `@sdks/urbackend-python/src/__init__.py`:
- Around line 1-47: The package is misconfigured to publish as top-level package
"src"; update packaging and imports so the public package is "urbackend": move
the module into a src/urbackend package (ensure __init__.py contains the
exported symbol UrBackendClient and the exception imports remain), then change
pyproject.toml to use setuptools package discovery with
[tool.setuptools.packages.find] where = ["src"] so the installer finds
"urbackend"; finally update the module docstring example (change "from src
import UrBackendClient" to "from urbackend import UrBackendClient") and adjust
any tests or internal imports that reference "src".
In `@sdks/urbackend-python/src/client.py`:
- Around line 196-202: connect() passes base straight to UrBackendHTTP which
causes AttributeError when base is None; mirror the __init__ behavior by
defaulting base = base or _DEFAULT_BASE before constructing UrBackendHTTP in
connect(), then use that normalized base when creating the new UrBackendHTTP
instance (update the call in connect() that currently creates UrBackendHTTP(...,
base=base, ...)); keep existing handling of _http, _extra_headers and ensure
_validate_api_key(api_key) remains called first.
In `@sdks/urbackend-python/src/storage.py`:
- Around line 81-130: The current upload reads entire content into memory
(file_bytes/file_size) before requesting a signed URL and PUT, which breaks for
large files; change logic in the upload function so that for Path inputs
(path_obj) you stat() to get size and open the file as an open binary handle
(rb) and pass that handle as data to requests.put, and for file-like inputs
(IOBase or hasattr(file,"read")) if the object is seekable use seek(0,2) to get
size then seek(0) to rewind and pass the handle directly as data; only keep
reading into bytes for bytes/bytearray inputs and continue using resolved_name,
content_type, signed_info (self._http.request), signed_url and file_path as
before so file_size is computed from stat()/seeked position and requests.put
streams the body instead of buffering into file_bytes.
In `@sdks/urbackend-python/test_sdk.py`:
- Around line 30-42: The test reads the base URL from URBACKEND_BASE but docs
and prior reviews reference URBACKEND_BASE_URL; pick a single canonical env var
(recommend URBACKEND_BASE_URL), update the code so the BASE (or rename to
BASE_URL) variable uses os.environ.get("URBACKEND_BASE_URL",
"https://api.ub.bitbros.in"), update the missing-vars check to include the
chosen name (e.g., "URBACKEND_BASE_URL" if required), and update README
"Contributing" and .env.example to document the exact env var name and purpose
so contributors export the correct variable.
- Line 55: The print statement uses an unnecessary f-string: change
print(f"Client created with default base URL") to a regular string print("Client
created with default base URL") to satisfy Ruff F541; also scan the same file
(test_sdk.py) for other prints using an unused f-prefix and remove the leading f
in those calls (e.g., any other print(...) statements with no placeholders).
- Around line 308-331: The two inline UrBackendClient(...) usages in the test
(the auth.login and auth.me calls) create requests.Session sockets that are
never closed; change them to either assign to a variable and call .close() after
the try/except or instantiate via a context manager if UrBackendClient supports
__enter__/__exit__ (e.g., with UrBackendClient(api_key=API_KEY, base=BASE) as
client: client.auth.login(...)). Ensure both occurrences around auth.login and
auth.me explicitly close the client (or use with) to avoid leaking HTTP
sessions.
---
Nitpick comments:
In `@sdks/urbackend-python/src/__init__.py`:
- Around line 48-56: The __all__ export list is unsorted which triggers lint
RUF022; sort the entries in the __all__ list alphabetically (e.g., AuthError,
NotFoundError, RateLimitError, StorageError, UrBackendClient, UrBackendError,
ValidationError) so the __all__ symbol contains a consistently ordered list and
satisfies the linter.
In `@sdks/urbackend-python/src/auth.py`:
- Around line 11-14: Consolidate and reorder the imports in auth.py: move the
stdlib import warnings above the package-relative imports, and merge the two
exception imports into a single line importing both AuthError and
UrBackendError; keep UrBackendHTTP import alongside the other relative imports
so imports are grouped as stdlib, third-party (if any), then local (e.g.,
warnings first, then from .http import UrBackendHTTP and from .exceptions import
AuthError, UrBackendError).
In `@sdks/urbackend-python/src/db.py`:
- Around line 17-32: The parameter name filter shadows the builtin and should be
renamed internally and consistently across _build_params, get_all, and count
(e.g., filter_ or where) while preserving the public API; update the function
signatures to accept the user-facing keyword but map it to the new internal name
(and adjust type hints and docstrings), then update all internal references and
any calls to _build_params/get_all/count to pass the renamed variable (or accept
**kwargs and pop "filter" into the internal name) so Ruff A002 is resolved
without changing external behavior.
- Around line 80-116: CollectionRef.find is missing the skip and expand
parameters that DatabaseModule.get_all accepts; update the find method signature
to accept skip: Optional[int] and expand: Optional[Union[str, List[str]]]
(matching get_all's types), add them to the docstring args, and forward them
into the call to self._db.get_all (i.e., pass skip=skip and expand=expand
alongside filter, sort, limit, page, populate, token) so the collection-level
API surface matches DatabaseModule.get_all.
In `@sdks/urbackend-python/src/http.py`:
- Around line 57-83: The storage endpoint check (the if "/api/storage" in
endpoint branch) is placed after status-specific branches so 401/403/404/429/400
from storage endpoints never become StorageError; fix by either moving the
storage check to run before the status-based returns or by wrapping the specific
status-created exceptions into StorageError (e.g., detect "/api/storage" in
endpoint at the top of this error-mapping function and return
StorageError(message, status, endpoint) for any status, or after creating the
specific exception, if endpoint contains "/api/storage" re-wrap/return
StorageError with the original exception details so all storage endpoint
failures are represented as StorageError).
In `@sdks/urbackend-python/src/mail.py`:
- Around line 89-110: Add a client-side guard in the send method to validate
that a template or raw body is provided before making the HTTP call: check that
at least one of template_name or template_id is set OR at least one of subject,
text, or html is set; if none are present, raise a ValueError (or a custom
exception) with a clear message and return early instead of calling
self._http.request("POST", "/api/mail/send", body=payload). This validation
should live in the same function that builds payload (the send method) so it
executes before invoking _http.request.
In `@sdks/urbackend-python/src/storage.py`:
- Around line 125-130: The PUT to the cloud uses a hard-coded 120s timeout which
can fail large uploads; make the timeout configurable by adding a parameter
(e.g., timeout: float = 120) to the upload() function signature and pass that
parameter into the requests.put call (which currently creates put_resp).
Alternatively, read a module-level default that callers can override and use it
when calling requests.put; update any callers/tests of upload() to
provide/expect the new parameter if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d635287-74c6-4ec5-90a9-153ffad8a290
📒 Files selected for processing (16)
README.mdsdks/urbackend-python/README.mdsdks/urbackend-python/pyproject.tomlsdks/urbackend-python/src/__init__.pysdks/urbackend-python/src/auth.pysdks/urbackend-python/src/client.pysdks/urbackend-python/src/db.pysdks/urbackend-python/src/exceptions.pysdks/urbackend-python/src/http.pysdks/urbackend-python/src/mail.pysdks/urbackend-python/src/storage.pysdks/urbackend-python/test_sdk.pysdks/urbackend-python/tests/test_auth.pysdks/urbackend-python/tests/test_db.pysdks/urbackend-python/tests/test_exceptions.pysdks/urbackend-python/tests/test_http.py
✅ Files skipped from review due to trivial changes (1)
- sdks/urbackend-python/pyproject.toml
Collaborator
|
Hello @Navyasree-ulava |
Contributor
Author
resolving......... |
yash-pouranik
approved these changes
Apr 26, 2026
Collaborator
yash-pouranik
left a comment
yash-pouranik
left a comment
There was a problem hiding this comment.
@Navyasree-ulava
okay thank you for the PR and Cooperating very well.
Merging now.
Hope to see more contributions from you.
Lastly Please leave a Star to the repo.
yash-pouranik
added
feature
Contributor
Author
|
Thank u so much! 😊 I’ve starred the repo ⭐ |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes Issue #100
This PR adds the official Python SDK (v0.1.0) for urBackend, enabling Python developers to interact with urBackend APIs for authentication and database operations.
The SDK mirrors the TypeScript SDK structure while keeping the implementation simple and easy to use. It includes full support for auth, CRUD operations, token handling, and error management.
Additionally, I added comprehensive testing (
test_sdk.py) covering real API usage, negative scenarios, and edge cases.Type of Change
Testing & Validation
Backend Verification:
npm testin thebackend/directorytest_sdk.py)Frontend Verification:
What’s Implemented
Client
connect(api_key, project_id, base_url)Auth
login,me,logoutDatabase
collection(name)find,insert,get_one,patch,delete,countInternal
x-api-key,Authorization)Testing Coverage
Tested against live API:
https://api.ub.bitbros.inCovers:
Auth flow (signup, login, logout)
Token auto-storage and reuse
Full CRUD operations
Query parameters (limit, sort, filter)
Negative cases:
Edge cases:
Token override behavior
All tests pass via
python test_sdk.py.✅ Checklist
Built with ❤️ for urBackend.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores