Open
Conversation
Label-based API, Python 3 only, no Paginator/InfoList. Uses v2.0 for device/variable CRUD, v1.6 for data ingestion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Feb 24, 2026
📝 Walkthrough🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Closed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
tests/test_http.py (1)
74-93: Missing test forputmethod.
HttpClient.put()is defined in_http.pybut has no corresponding test here, unlikeget,post,patch, anddelete.Add the missing test
`@patch`("ubidots._http.HttpClient._request") def test_delete(self, mock_req, http): http.delete("http://example.com") mock_req.assert_called_once_with("DELETE", "http://example.com") + + `@patch`("ubidots._http.HttpClient._request") + def test_put(self, mock_req, http): + http.put("http://example.com", json={"k": "v"}) + mock_req.assert_called_once_with("PUT", "http://example.com", json={"k": "v"})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_http.py` around lines 74 - 93, Add a missing unit test for HttpClient.put by adding a method in the TestHttpMethods test class that patches ubidots._http.HttpClient._request, calls http.put("http://example.com", json={"k": "v"}) (matching the style of test_post/test_patch), and asserts mock_req.assert_called_once_with("PUT", "http://example.com", json={"k": "v"}); use the same patch target and test fixture names (TestHttpMethods, HttpClient._request, and http) as the other tests to keep consistency.ubidots/__init__.py (1)
1-12: Consider defining__all__for explicit public API surface.As a public library, defining
__all__ensuresfrom ubidots import *only exposes intended symbols and serves as self-documentation of the public API.Suggested addition
__version__ = "2.0.0" +__all__ = [ + "Ubidots", + "UbidotsError", + "UbidotsError400", + "UbidotsError404", + "UbidotsError500", + "UbidotsForbiddenError", + "UbidotsHTTPError", + "UbidotsInvalidInputError", +] + from .client import Ubidots from .exceptions import (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/__init__.py` around lines 1 - 12, Add an explicit __all__ list to this module so the public API is well-defined; include "__version__", "Ubidots" and all exported exception names (UbidotsError, UbidotsError400, UbidotsError404, UbidotsError500, UbidotsForbiddenError, UbidotsHTTPError, UbidotsInvalidInputError) in the __all__ tuple placed near the top of the file next to the existing imports; ensure the exact symbol names match those imported so wildcard imports only expose these intended symbols.ubidots/_http.py (2)
14-23: Consider addingclose()/ context-manager support to avoid resource leaks.
requests.Sessionholds a connection pool. In long-running applications, unclosed sessions can leak connections. Adding aclose()method (and optionally__enter__/__exit__) lets callers manage the lifecycle.Suggested addition
class HttpClient: """Thin wrapper around requests.Session with Ubidots auth and error handling.""" def __init__(self, token, timeout=DEFAULT_TIMEOUT): self.timeout = timeout self.session = requests.Session() self.session.headers.update({ "X-Auth-Token": token, "Content-Type": "application/json", }) + + def close(self): + self.session.close() + + def __enter__(self): + return self + + def __exit__(self, *exc): + self.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/_http.py` around lines 14 - 23, The HttpClient currently creates a requests.Session in __init__ but provides no way to close it; add a public close(self) method that calls self.session.close() and implement context-manager support by adding __enter__(self) returning self and __exit__(self, exc_type, exc, tb) that calls close(); update any existing cleanup (or add a __del__ fallback) so users can use "with HttpClient(...)" or explicitly call close() to avoid leaking the underlying connection pool referenced by the session attribute in HttpClient.
53-56: Narrow the bareExceptioncatch to specific exceptions.Static analysis (Ruff BLE001) flags this.
response.textcan only realistically fail with encoding issues. CatchingExceptioncould mask unexpected bugs.Proposed fix
try: detail = response.text - except Exception: + except (AttributeError, UnicodeDecodeError): detail = ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/_http.py` around lines 53 - 56, The code currently catches a bare Exception when accessing response.text; narrow this to the specific decoding/HTTP errors that can occur (e.g., catch UnicodeDecodeError and requests.exceptions.RequestException or requests.exceptions.ContentDecodingError) so other bugs aren't masked. Update the try/except around response.text in ubidots/_http.py to import and catch the specific exceptions (e.g., except (UnicodeDecodeError, requests.exceptions.RequestException):) and set detail = "" in that branch.setup.py (1)
21-33: Consider upgrading Python minimum version to 3.9+ and add Python 3.13 classifier.Python 3.8 reached end-of-life on October 7, 2024, and Python 3.13 was released the same day. For a fresh v2.0 release, update
python_requires=">=3.9"(or>=3.10) and add the3.13classifier to reflect current platform support.Suggested update
- python_requires=">=3.8", + python_requires=">=3.9", classifiers=[ "Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", "License :: OSI Approved :: MIT License", "Operating System :: OS Independent", "Programming Language :: Python", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.py` around lines 21 - 33, Update the package metadata to require a newer Python and advertise 3.13: change the python_requires value from ">=3.8" to ">=3.9" (or ">=3.10" if you prefer stricter minimum) and add the classifier "Programming Language :: Python :: 3.13" to the classifiers list so packaging tools and indexes reflect support for Python 3.13; ensure the symbols python_requires and the classifiers array in setup.py are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ubidots/_http.py`:
- Around line 40-46: In _request, calling response.json() can raise
JSONDecodeError for non-JSON bodies (e.g., proxy HTML) even on 2xx responses;
update the ubidots._http._request method to guard the parse by checking
response.headers.get("Content-Type") for "application/json" (or try/except
json.JSONDecodeError) around response.json(), and on failure either return
response.text (or None) or raise a clearer exception including status_code and
response.text; keep the existing _raise_for_status call and ensure
session.request usage and return contract are preserved.
In `@ubidots/client.py`:
- Around line 29-34: The create_device function currently builds body as
{"label": label, **fields} which lets a label inside **fields override the
explicit label; change the merge order so the explicit argument wins by copying
fields first and then setting body["label"] = label (or by using {**fields,
"label": label}), then add name if provided and call self._http.post(url,
json=body); apply the same fix to the other similar function referenced at lines
54-59 so explicit label/name parameters cannot be overridden by **fields.
In `@ubidots/exceptions.py`:
- Around line 28-32: UbidotsError500 currently hardcodes 500 and thus loses the
real 5xx status; change its signature to accept an optional status_code (e.g.,
def __init__(self, detail="", status_code=500)) and pass that status_code to
super().__init__(status_code, detail) so the actual status (502/503/etc.) is
preserved, then update callers in _http.py that raise UbidotsError500 to pass
the actual status (e.g., raise UbidotsError500(detail, status_code=status)).
- Around line 35-39: UbidotsForbiddenError currently hardcodes status_code=403
which overwrites a real 401; change its constructor
(UbidotsForbiddenError.__init__) to accept an optional status_code parameter
(default 403) and pass that through to the base UbidotsHTTPError so callers (and
_http.py when raising UbidotsForbiddenError(detail, status_code=status)) can
preserve the original status; update any instantiation sites accordingly to
supply the real status when available.
---
Nitpick comments:
In `@setup.py`:
- Around line 21-33: Update the package metadata to require a newer Python and
advertise 3.13: change the python_requires value from ">=3.8" to ">=3.9" (or
">=3.10" if you prefer stricter minimum) and add the classifier "Programming
Language :: Python :: 3.13" to the classifiers list so packaging tools and
indexes reflect support for Python 3.13; ensure the symbols python_requires and
the classifiers array in setup.py are updated accordingly.
In `@tests/test_http.py`:
- Around line 74-93: Add a missing unit test for HttpClient.put by adding a
method in the TestHttpMethods test class that patches
ubidots._http.HttpClient._request, calls http.put("http://example.com",
json={"k": "v"}) (matching the style of test_post/test_patch), and asserts
mock_req.assert_called_once_with("PUT", "http://example.com", json={"k": "v"});
use the same patch target and test fixture names (TestHttpMethods,
HttpClient._request, and http) as the other tests to keep consistency.
In `@ubidots/__init__.py`:
- Around line 1-12: Add an explicit __all__ list to this module so the public
API is well-defined; include "__version__", "Ubidots" and all exported exception
names (UbidotsError, UbidotsError400, UbidotsError404, UbidotsError500,
UbidotsForbiddenError, UbidotsHTTPError, UbidotsInvalidInputError) in the
__all__ tuple placed near the top of the file next to the existing imports;
ensure the exact symbol names match those imported so wildcard imports only
expose these intended symbols.
In `@ubidots/_http.py`:
- Around line 14-23: The HttpClient currently creates a requests.Session in
__init__ but provides no way to close it; add a public close(self) method that
calls self.session.close() and implement context-manager support by adding
__enter__(self) returning self and __exit__(self, exc_type, exc, tb) that calls
close(); update any existing cleanup (or add a __del__ fallback) so users can
use "with HttpClient(...)" or explicitly call close() to avoid leaking the
underlying connection pool referenced by the session attribute in HttpClient.
- Around line 53-56: The code currently catches a bare Exception when accessing
response.text; narrow this to the specific decoding/HTTP errors that can occur
(e.g., catch UnicodeDecodeError and requests.exceptions.RequestException or
requests.exceptions.ContentDecodingError) so other bugs aren't masked. Update
the try/except around response.text in ubidots/_http.py to import and catch the
specific exceptions (e.g., except (UnicodeDecodeError,
requests.exceptions.RequestException):) and set detail = "" in that branch.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
README.mdREADME.rstsetup.pytests.pytests/__init__.pytests/test_client.pytests/test_http.pyubidots/__init__.pyubidots/_endpoints.pyubidots/_http.pyubidots/apiclient.pyubidots/client.pyubidots/exceptions.py
💤 Files with no reviewable changes (3)
- README.rst
- tests.py
- ubidots/apiclient.py
Comment on lines
+40
to
+46
| def _request(self, method, url, **kwargs): | ||
| kwargs.setdefault("timeout", self.timeout) | ||
| response = self.session.request(method, url, **kwargs) | ||
| self._raise_for_status(response) | ||
| if response.status_code == 204 or not response.content: | ||
| return None | ||
| return response.json() |
There was a problem hiding this comment.
response.json() will raise JSONDecodeError if the response body isn't valid JSON.
If a proxy or load balancer returns a non-JSON body (e.g., an HTML error page) with a 200-level status code and non-empty content, Line 46 will raise an unhandled JSONDecodeError. Consider wrapping with a try/except or checking the Content-Type header.
Proposed fix
def _request(self, method, url, **kwargs):
kwargs.setdefault("timeout", self.timeout)
response = self.session.request(method, url, **kwargs)
self._raise_for_status(response)
if response.status_code == 204 or not response.content:
return None
- return response.json()
+ try:
+ return response.json()
+ except ValueError:
+ return response.text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ubidots/_http.py` around lines 40 - 46, In _request, calling response.json()
can raise JSONDecodeError for non-JSON bodies (e.g., proxy HTML) even on 2xx
responses; update the ubidots._http._request method to guard the parse by
checking response.headers.get("Content-Type") for "application/json" (or
try/except json.JSONDecodeError) around response.json(), and on failure either
return response.text (or None) or raise a clearer exception including
status_code and response.text; keep the existing _raise_for_status call and
ensure session.request usage and return contract are preserved.
Comment on lines
+29
to
+34
| def create_device(self, *, label, name=None, **fields): | ||
| url = self._endpoints.devices_url() | ||
| body = {"label": label, **fields} | ||
| if name is not None: | ||
| body["name"] = name | ||
| return self._http.post(url, json=body) |
There was a problem hiding this comment.
Prevent **fields from overriding the explicit label argument.
If callers pass label inside **fields, it silently wins over the explicit label parameter. Prefer making the explicit argument authoritative.
🔧 Proposed fix
- body = {"label": label, **fields}
+ body = {**fields, "label": label}
if name is not None:
body["name"] = name- body = {"label": label, **fields}
+ body = {**fields, "label": label}
if name is not None:
body["name"] = nameAlso applies to: 54-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ubidots/client.py` around lines 29 - 34, The create_device function currently
builds body as {"label": label, **fields} which lets a label inside **fields
override the explicit label; change the merge order so the explicit argument
wins by copying fields first and then setting body["label"] = label (or by using
{**fields, "label": label}), then add name if provided and call
self._http.post(url, json=body); apply the same fix to the other similar
function referenced at lines 54-59 so explicit label/name parameters cannot be
overridden by **fields.
Comment on lines
+28
to
+32
| class UbidotsError500(UbidotsHTTPError): | ||
| """Internal server error.""" | ||
|
|
||
| def __init__(self, detail=""): | ||
| super().__init__(500, detail) |
There was a problem hiding this comment.
Same issue: UbidotsError500 hardcodes 500, losing the actual 5xx code (502, 503, etc.).
_http.py raises UbidotsError500 for any status >= 500, but the exception always reports status_code=500.
Proposed fix
class UbidotsError500(UbidotsHTTPError):
"""Internal server error."""
- def __init__(self, detail=""):
- super().__init__(500, detail)
+ def __init__(self, detail="", status_code=500):
+ super().__init__(status_code, detail)Then in _http.py:
raise UbidotsError500(detail, status_code=status)📝 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
| class UbidotsError500(UbidotsHTTPError): | |
| """Internal server error.""" | |
| def __init__(self, detail=""): | |
| super().__init__(500, detail) | |
| class UbidotsError500(UbidotsHTTPError): | |
| """Internal server error.""" | |
| def __init__(self, detail="", status_code=500): | |
| super().__init__(status_code, detail) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ubidots/exceptions.py` around lines 28 - 32, UbidotsError500 currently
hardcodes 500 and thus loses the real 5xx status; change its signature to accept
an optional status_code (e.g., def __init__(self, detail="", status_code=500))
and pass that status_code to super().__init__(status_code, detail) so the actual
status (502/503/etc.) is preserved, then update callers in _http.py that raise
UbidotsError500 to pass the actual status (e.g., raise UbidotsError500(detail,
status_code=status)).
Comment on lines
+35
to
+39
| class UbidotsForbiddenError(UbidotsHTTPError): | ||
| """Authentication or authorization failure (401/403).""" | ||
|
|
||
| def __init__(self, detail=""): | ||
| super().__init__(403, detail) |
There was a problem hiding this comment.
UbidotsForbiddenError hardcodes status_code=403, losing the original 401 status.
When _http.py raises UbidotsForbiddenError for a 401 response, the exception's status_code will be 403, not 401. Users inspecting exc.status_code will see incorrect information.
Consider accepting an optional status_code parameter:
Proposed fix
class UbidotsForbiddenError(UbidotsHTTPError):
"""Authentication or authorization failure (401/403)."""
- def __init__(self, detail=""):
- super().__init__(403, detail)
+ def __init__(self, detail="", status_code=403):
+ super().__init__(status_code, detail)Then in _http.py:
raise UbidotsForbiddenError(detail, status_code=status)📝 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
| class UbidotsForbiddenError(UbidotsHTTPError): | |
| """Authentication or authorization failure (401/403).""" | |
| def __init__(self, detail=""): | |
| super().__init__(403, detail) | |
| class UbidotsForbiddenError(UbidotsHTTPError): | |
| """Authentication or authorization failure (401/403).""" | |
| def __init__(self, detail="", status_code=403): | |
| super().__init__(status_code, detail) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ubidots/exceptions.py` around lines 35 - 39, UbidotsForbiddenError currently
hardcodes status_code=403 which overwrites a real 401; change its constructor
(UbidotsForbiddenError.__init__) to accept an optional status_code parameter
(default 403) and pass that through to the base UbidotsHTTPError so callers (and
_http.py when raising UbidotsForbiddenError(detail, status_code=status)) can
preserve the original status; update any instantiation sites accordingly to
supply the real status when available.
|
Note Docstrings generation - SKIPPED |
Docstrings generation was requested by @AgustinPelaez. * #26 (comment) The following files were modified: * `setup.py` * `tests/test_client.py` * `tests/test_http.py` * `ubidots/_endpoints.py` * `ubidots/_http.py` * `ubidots/client.py` * `ubidots/exceptions.py`
Merged
📝 Add docstrings to `v2.0`
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
ubidots/exceptions.py (2)
50-60:UbidotsError500hardcodes status 500, masking actual 5xx codes (502, 503, etc.).This was raised in a previous review and remains unaddressed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/exceptions.py` around lines 50 - 60, UbidotsError500 currently hardcodes HTTP status 500 in its __init__, which masks other 5xx codes (e.g., 502, 503); update the UbidotsError500.__init__ signature to accept an optional status parameter (default 500) and pass that status to the UbidotsHTTPError(super) call (keeping the existing detail parameter) so callers can construct specific 5xx errors; reference the UbidotsError500 class and its __init__ and ensure compatibility with UbidotsHTTPError's constructor.
63-73:UbidotsForbiddenErrorhardcodes 403, losing the original 401 status.This was raised in a previous review and remains unaddressed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/exceptions.py` around lines 63 - 73, UbidotsForbiddenError currently hardcodes 403 which discards incoming 401 responses; change its signature to accept an optional status parameter (default 403) and pass that status to the parent UbidotsHTTPError so the original status (e.g., 401) is preserved; update the constructor of UbidotsForbiddenError to use the new status arg and ensure callers that create this exception pass through the HTTP status when available.ubidots/client.py (1)
64-80:**fieldscan silently override the explicitlabelparameter.This was raised in a previous review and remains unaddressed. The same issue exists in
create_variableat Line 150.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/client.py` around lines 64 - 80, The create_device function (and similarly create_variable) currently builds the request body by merging **fields before the explicit "label" (and "name"), allowing callers to silently override those required parameters; fix by preventing shadowing—either validate and raise a ValueError if "label" (and "name" for create_device) exist in **fields, or construct the body so the explicit keys overwrite fields (e.g., merge fields first then set body["label"]=label and body["name"]=name if provided) and apply the same change to create_variable to ensure explicit parameters cannot be overridden by **fields.ubidots/_http.py (1)
114-116:response.json()can raiseJSONDecodeErroron non-JSON bodies.If a proxy or CDN returns a non-JSON body (e.g., HTML error page) with a 2xx status and non-empty content, this will raise an unhandled
json.JSONDecodeError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/_http.py` around lines 114 - 116, The current block that returns response.json() after checking response.status_code and response.content can raise json.JSONDecodeError for non-JSON bodies; wrap the call to response.json() in a try/except catching json.JSONDecodeError (or ValueError for compatibility), and on error either return a safe value (e.g., response.text or None) or raise a controlled exception that includes response.status_code and response.content/response.text for diagnostics; update the code around the response.status_code / response.content check so response.json() is invoked inside that try/except and ensure json.JSONDecodeError is imported where needed.
🧹 Nitpick comments (5)
ubidots/_http.py (1)
136-139: Narrow the bareexcept Exceptionto specific exceptions.Ruff flags
BLE001here. While this is a defensive fallback forresponse.text, catching all exceptions is overly broad.response.textcan realistically only raise encoding-related errors.Proposed fix
try: detail = response.text - except Exception: + except (AttributeError, UnicodeDecodeError): detail = ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/_http.py` around lines 136 - 139, The try/except that assigns detail = response.text should avoid a bare except; narrow it to the likely decoding errors (e.g., catch UnicodeDecodeError and requests.exceptions.ContentDecodingError) so only those are swallowed and others propagate. Update the block in ubidots/_http.py that currently wraps response.text to except (UnicodeDecodeError, ContentDecodingError) and set detail = "" in that except, adding an import for ContentDecodingError from requests.exceptions if needed.tests/test_http.py (1)
90-109: Missing test forput()method.
HttpClientexposes aputmethod (seeubidots/_http.pyLines 60–71), butTestHttpMethodsonly coversget,post,patch, anddelete. Consider adding a test to verifyputdelegates correctly.Add a test for put
`@patch`("ubidots._http.HttpClient._request") def test_delete(self, mock_req, http): http.delete("http://example.com") mock_req.assert_called_once_with("DELETE", "http://example.com") + + `@patch`("ubidots._http.HttpClient._request") + def test_put(self, mock_req, http): + http.put("http://example.com", json={"k": "v"}) + mock_req.assert_called_once_with("PUT", "http://example.com", json={"k": "v"})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_http.py` around lines 90 - 109, Add a missing unit test for HttpClient.put: in the TestHttpMethods class add a test method (similar to test_post/test_patch) that patches ubidots._http.HttpClient._request, calls http.put("http://example.com", json={"k":"v"}) (or other payload), and asserts mock_req.assert_called_once_with("PUT", "http://example.com", json={"k":"v"}); reference HttpClient._request, HttpClient.put and TestHttpMethods to mirror the existing test patterns.setup.py (2)
29-29: Excludetests.*sub-packages as well.
find_packages(exclude=["tests"])only excludes the top-leveltestspackage. Any nested sub-packages (e.g.,tests.integration) would still be included in the distribution.Proposed fix
- packages=find_packages(exclude=["tests"]), + packages=find_packages(exclude=["tests", "tests.*"]),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.py` at line 29, The setup.py currently calls find_packages(exclude=["tests"]) which only omits the top-level tests package; update the find_packages call (the symbol find_packages in setup.py) to exclude both the top-level and any subpackages by using exclude=["tests", "tests.*"] so nested test packages like tests.integration are not included in the distribution.
37-42: Consider adding the Python 3.13 classifier.Python 3.13 was released in October 2024. If the library is compatible (and
python_requires=">=3.8"suggests it should be), adding the classifier would be appropriate for completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.py` around lines 37 - 42, Add the Python 3.13 trove classifier to the package metadata so the classifiers list in setup.py reflects support for 3.13; update the list that currently contains "Programming Language :: Python :: 3.12" to also include "Programming Language :: Python :: 3.13" (and verify the existing python_requires value still matches the stated compatibility).ubidots/client.py (1)
228-235: Inconsistent docstring style onget_last_value.Line 232 uses
@returns(Javadoc-style) while all other methods in this class use theReturns:(Google/NumPy-style) convention.Proposed fix
def get_last_value(self, device_label, variable_label): """ Fetches the most recent value for a variable on a device. - - `@returns` dict: The API response containing the latest value record for the specified variable. + + Parameters: + device_label (str): Label of the device containing the variable. + variable_label (str): Label of the variable whose latest value is requested. + + Returns: + dict: The API response containing the latest value record for the specified variable. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ubidots/client.py` around lines 228 - 235, The get_last_value method's docstring uses Javadoc-style "@returns" which is inconsistent with the rest of the class; update the docstring for get_last_value to use the same "Returns:" (Google/NumPy-style) block as other methods and keep the same return description/type (e.g., "Returns: dict: The API response containing the latest value record for the specified variable."). Ensure the function name get_last_value and its return description remain unchanged semantically while switching the annotation style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ubidots/client.py`:
- Around line 239-257: _pagate currently assumes self._http.get(...) returns a
dict; if it returns None (e.g., 204/empty) the call to data.get(...) will raise
AttributeError. Update the _paginate method to guard against None by treating a
None response as an empty dict before accessing "results" and "next" (i.e.,
after calling self._http.get(url, params=params) assign data = data or {}), then
continue with results.extend(data.get("results", [])) and url =
data.get("next"); keep the existing params reset logic (params = {}) for
subsequent pages.
---
Duplicate comments:
In `@ubidots/_http.py`:
- Around line 114-116: The current block that returns response.json() after
checking response.status_code and response.content can raise
json.JSONDecodeError for non-JSON bodies; wrap the call to response.json() in a
try/except catching json.JSONDecodeError (or ValueError for compatibility), and
on error either return a safe value (e.g., response.text or None) or raise a
controlled exception that includes response.status_code and
response.content/response.text for diagnostics; update the code around the
response.status_code / response.content check so response.json() is invoked
inside that try/except and ensure json.JSONDecodeError is imported where needed.
In `@ubidots/client.py`:
- Around line 64-80: The create_device function (and similarly create_variable)
currently builds the request body by merging **fields before the explicit
"label" (and "name"), allowing callers to silently override those required
parameters; fix by preventing shadowing—either validate and raise a ValueError
if "label" (and "name" for create_device) exist in **fields, or construct the
body so the explicit keys overwrite fields (e.g., merge fields first then set
body["label"]=label and body["name"]=name if provided) and apply the same change
to create_variable to ensure explicit parameters cannot be overridden by
**fields.
In `@ubidots/exceptions.py`:
- Around line 50-60: UbidotsError500 currently hardcodes HTTP status 500 in its
__init__, which masks other 5xx codes (e.g., 502, 503); update the
UbidotsError500.__init__ signature to accept an optional status parameter
(default 500) and pass that status to the UbidotsHTTPError(super) call (keeping
the existing detail parameter) so callers can construct specific 5xx errors;
reference the UbidotsError500 class and its __init__ and ensure compatibility
with UbidotsHTTPError's constructor.
- Around line 63-73: UbidotsForbiddenError currently hardcodes 403 which
discards incoming 401 responses; change its signature to accept an optional
status parameter (default 403) and pass that status to the parent
UbidotsHTTPError so the original status (e.g., 401) is preserved; update the
constructor of UbidotsForbiddenError to use the new status arg and ensure
callers that create this exception pass through the HTTP status when available.
---
Nitpick comments:
In `@setup.py`:
- Line 29: The setup.py currently calls find_packages(exclude=["tests"]) which
only omits the top-level tests package; update the find_packages call (the
symbol find_packages in setup.py) to exclude both the top-level and any
subpackages by using exclude=["tests", "tests.*"] so nested test packages like
tests.integration are not included in the distribution.
- Around line 37-42: Add the Python 3.13 trove classifier to the package
metadata so the classifiers list in setup.py reflects support for 3.13; update
the list that currently contains "Programming Language :: Python :: 3.12" to
also include "Programming Language :: Python :: 3.13" (and verify the existing
python_requires value still matches the stated compatibility).
In `@tests/test_http.py`:
- Around line 90-109: Add a missing unit test for HttpClient.put: in the
TestHttpMethods class add a test method (similar to test_post/test_patch) that
patches ubidots._http.HttpClient._request, calls http.put("http://example.com",
json={"k":"v"}) (or other payload), and asserts
mock_req.assert_called_once_with("PUT", "http://example.com", json={"k":"v"});
reference HttpClient._request, HttpClient.put and TestHttpMethods to mirror the
existing test patterns.
In `@ubidots/_http.py`:
- Around line 136-139: The try/except that assigns detail = response.text should
avoid a bare except; narrow it to the likely decoding errors (e.g., catch
UnicodeDecodeError and requests.exceptions.ContentDecodingError) so only those
are swallowed and others propagate. Update the block in ubidots/_http.py that
currently wraps response.text to except (UnicodeDecodeError,
ContentDecodingError) and set detail = "" in that except, adding an import for
ContentDecodingError from requests.exceptions if needed.
In `@ubidots/client.py`:
- Around line 228-235: The get_last_value method's docstring uses Javadoc-style
"@returns" which is inconsistent with the rest of the class; update the
docstring for get_last_value to use the same "Returns:" (Google/NumPy-style)
block as other methods and keep the same return description/type (e.g.,
"Returns: dict: The API response containing the latest value record for the
specified variable."). Ensure the function name get_last_value and its return
description remain unchanged semantically while switching the annotation style.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
setup.pytests/test_client.pytests/test_http.pyubidots/_endpoints.pyubidots/_http.pyubidots/client.pyubidots/exceptions.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_client.py
Comment on lines
+239
to
+257
| def _paginate(self, url, params): | ||
| """ | ||
| Aggregate paginated GET responses from an API endpoint into a single list. | ||
|
|
||
| Parameters: | ||
| url (str): Initial endpoint URL to request. | ||
| params (Mapping): Query parameters applied to the first request; subsequent pages follow `next` URLs and ignore these params. | ||
|
|
||
| Returns: | ||
| list: Concatenated list of items from each page's `"results"` field (empty list if none). | ||
| """ | ||
| results = [] | ||
| params = dict(params) | ||
| while url: | ||
| data = self._http.get(url, params=params) | ||
| results.extend(data.get("results", [])) | ||
| url = data.get("next") | ||
| params = {} # next URL already contains query params | ||
| return results |
There was a problem hiding this comment.
_paginate will crash with AttributeError if _http.get returns None.
_http.get returns None for 204 responses or empty content (see _http.py Line 114). If a paginated endpoint ever returns such a response, data.get("results", []) on Line 254 will raise AttributeError: 'NoneType' object has no attribute 'get'.
Proposed fix
def _paginate(self, url, params):
results = []
params = dict(params)
while url:
data = self._http.get(url, params=params)
+ if not data:
+ break
results.extend(data.get("results", []))
url = data.get("next")
params = {} # next URL already contains query params
return results🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ubidots/client.py` around lines 239 - 257, _pagate currently assumes
self._http.get(...) returns a dict; if it returns None (e.g., 204/empty) the
call to data.get(...) will raise AttributeError. Update the _paginate method to
guard against None by treating a None response as an empty dict before accessing
"results" and "next" (i.e., after calling self._http.get(url, params=params)
assign data = data or {}), then continue with results.extend(data.get("results",
[])) and url = data.get("next"); keep the existing params reset logic (params =
{}) for subsequent pages.
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
1 participant
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.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores