Fixes and style #425
Fixes and style #425
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @juancarlospaco - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -71,14 +71,14 @@ async def execute(self) -> APIResponse[_ReturnT]: | |||
| if self.headers.get( | |||
| "Accept" | |||
| ) and "application/vnd.pgrst.plan" in self.headers.get("Accept"): | |||
| if not "+json" in self.headers.get("Accept"): | |||
There was a problem hiding this comment.
suggestion (code_refinement): Improved readability with 'not in' for membership test.
| if not "+json" in self.headers.get("Accept"): | |
| if "+json" not in self.headers.get("Accept"): |
| return body | ||
| return APIResponse[_ReturnT].from_http_request_response(r) | ||
| else: | ||
| raise APIError(r.json()) | ||
| except ValidationError as e: | ||
| raise APIError(r.json()) from e | ||
| except JSONDecodeError as e: |
There was a problem hiding this comment.
suggestion (code_refinement): Removal of unused exception variable 'e' is a good practice.
| except JSONDecodeError as e: | |
| except JSONDecodeError: | |
| raise APIError(generate_default_error_message(r)) |
| return body | ||
| return APIResponse[_ReturnT].from_http_request_response(r) | ||
| else: | ||
| raise APIError(r.json()) | ||
| except ValidationError as e: | ||
| raise APIError(r.json()) from e | ||
| except JSONDecodeError as e: |
There was a problem hiding this comment.
suggestion (code_refinement): Good removal of the unused variable 'e' in exception handling.
| @@ -196,7 +196,7 @@ def from_http_request_response( | |||
| ) -> Self: | |||
| try: | |||
| data = request_response.json() | |||
| except JSONDecodeError as e: | |||
There was a problem hiding this comment.
suggestion (code_refinement): Efficient use of exception handling by removing unused variable 'e'.
| except JSONDecodeError as e: | |
| except JSONDecodeError: | |
| raise APIError(generate_default_error_message(r)) |
| @@ -71,14 +71,14 @@ def execute(self) -> APIResponse[_ReturnT]: | |||
| if self.headers.get( | |||
| "Accept" | |||
| ) and "application/vnd.pgrst.plan" in self.headers.get("Accept"): | |||
| if not "+json" in self.headers.get("Accept"): | |||
There was a problem hiding this comment.
suggestion (code_refinement): Consistency in using 'not in' for membership checks enhances code readability.
| if not "+json" in self.headers.get("Accept"): | |
| if "+json" not in self.headers.get("Accept"): |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #425 +/- ##
==========================================
+ Coverage 95.72% 95.91% +0.18%
==========================================
Files 28 28
Lines 1639 1715 +76
==========================================
+ Hits 1569 1645 +76
Misses 70 70 ☔ View full report in Codecov by Sentry. |
What kind of change does this PR introduce?