Fix buckinghamshire council endpoint#1773
Conversation
WalkthroughModifies BuckinghamshireCouncil API requests from POST to GET with parameterized URL format. Updates User-Agent header, adjusts error handling to truncate response text for consistency, and applies minor formatting changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1773 +/- ##
=======================================
Coverage 86.81% 86.81%
=======================================
Files 9 9
Lines 1138 1138
=======================================
Hits 988 988
Misses 150 150 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
uk_bin_collection/uk_bin_collection/councils/BuckinghamshireCouncil.py (2)
76-84: GET migration and URL construction look fine; considerparamsfor clarityThe switch to
session.getwithP_PARAMETER={encoded_input}is consistent with a GET-style API, and using a hex string here is URL-safe. For readability and to avoid manual query-string construction, you could instead rely onparams:response = session.get( "https://itouchvision.app/portal/itouchvision/kmbd/collectionDay", headers=headers, params={"P_PARAMETER": encoded_input}, )This makes it a bit clearer what’s part of the base URL vs query parameters.
86-99: Error handling is solid; optional refactor to de-duplicate long messagesThe status-code check plus the “hex-like” validation give good early failure signals and concise, truncated context from the response. The two
ValueErrorraises (Lines 88-90 and 96-98) share a very similar long message shape, which is exactly what Ruff’s TRY003/TRY301 are flagging.If you want to quiet those hints and tighten things up, consider extracting a small helper that formats these API errors (including the
response.text[:200]snippet) and reuse it in both branches, or define a small custom exception that encapsulates the formatting. Functionally you’re in good shape either way; this would just reduce duplication and keep the call-site strings shorter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
uk_bin_collection/uk_bin_collection/councils/BuckinghamshireCouncil.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
uk_bin_collection/uk_bin_collection/councils/BuckinghamshireCouncil.py
88-90: Abstract raise to an inner function
(TRY301)
88-90: Avoid specifying long messages outside the exception class
(TRY003)
96-98: Abstract raise to an inner function
(TRY301)
96-98: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests (3.12, 1.8.4)
- GitHub Check: Run Integration Tests (3.12, 1.8.4)
|
Why was this closed? |
|
Merged into January release |
Endpoint changed from POST request to GET
#1767
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.