Dec release#1754
Conversation
BREAKING CHANGE fix: #1693 This scraper now uses postcode and house number instead of UPRN. It also requires a Selenium browser
Bin dates have changed so this needs to be updated to pull the new data
The Merton Council website has changed from myneighbourhood.merton.gov.uk to fixmystreet.merton.gov.uk. Updated the script to: - Use the new FixMyStreet-based website - Poll the page until JavaScript loads collection data - Accept UPRN parameter instead of full URL - Skip booking services (Bulky waste, Garden waste) Fixes #1664 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Prevent empty string edge case with `split()` by gating on truthiness of `line.strip)`
This commit addresses all issues raised in the CodeRabbit review: 1. Replace star import with explicit imports - Changed from `import *` to explicit imports of `datetime` and `date_format` - Eliminates namespace pollution and F405 static analysis warnings 2. Add UPRN format validation - Validates that UPRN contains only numeric characters - Prevents potential security/stability issues from invalid input 3. Fix year-boundary date parsing bug - Checks if parsed date is in the past and adjusts to next year - Fixes December-to-January collection date parsing issues 4. Parameterize polling configuration - Moved hardcoded values to class constants (MAX_POLLING_ATTEMPTS, POLLING_SLEEP_SECONDS) - Improves flexibility for varying network conditions 5. Add comprehensive docstrings - Added detailed class and method docstrings - Includes Args, Returns, Raises, Examples, and Notes sections - Significantly improves docstring coverage All changes maintain backward compatibility while improving code quality, security, and maintainability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
It's definitely a bit of an edge case, but being a little more tolerant won't hurt.
... and handle request exceptions
* Amend loop variable for readability * Introduce temp variable to build list of collection data with sortable datetime field * Use dict comprehension to build final payload in order to avoid mutating dicts while looping
Signed-off-by: Tim Collins <tim@thecollins.team> formatting Signed-off-by: Tim Collins <tim@thecollins.team> chore: trigger CI for SouthamptonCityCouncil fix
Signed-off-by: Tim Collins <tim@thecollins.team>
chore: bump urllib3 from 2.5.0 to 2.6.0
fix: Norwich after u1 to ul change
|
Warning Rate limit exceeded@robbrad has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 5 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (7)
WalkthroughThis PR updates GitHub Actions workflows to use Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
❌ 7 Tests Failed:
View the full list of 7 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (7)
uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py (1)
39-40: Same date concern applies to Round B.These November 2025 dates have the same issue as noted in lines 36-37. Ensure the start dates align with your intended collection schedule.
.github/workflows/wiki.yml (1)
25-25: Consistent Actions upgrade across workflows.Same v6 upgrade as in other workflow files—refer to the verification request in
validate-release-ready.yml..github/workflows/hacs_validation.yml (1)
14-14: Consistent Actions upgrade.Same v6 upgrade—refer to verification in
validate-release-ready.yml..github/workflows/lint.yml (1)
19-19: Consistent Actions upgrade.Same v6 upgrade—refer to verification in
validate-release-ready.yml..github/workflows/codeql-analysis.yml (1)
50-50: Consistent Actions upgrade.Same v6 upgrade—refer to verification in
validate-release-ready.yml..github/workflows/bump.yml (1)
22-22: Consistent Actions upgrade.Same v6 upgrade—refer to verification in
validate-release-ready.yml..github/workflows/ha_compatibility_test.yml (1)
56-56: Consistent Actions upgrade across both jobs.Both checkout steps upgraded to v6—refer to verification in
validate-release-ready.yml.Also applies to: 230-230
🧹 Nitpick comments (18)
uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py (1)
38-40: User‑agent addition andcreate_webdrivercall look correct; consider fixing Ruff F405 by avoidingimport *.
- The new
user_agentassignment and the callcreate_webdriver(web_driver, headless, user_agent, __name__)match thecreate_webdriversignature and should behave as intended.- Ruff’s F405 on
create_webdriveris due tofrom uk_bin_collection.uk_bin_collection.common import *(name only guaranteed via star import). To keep linters happy and make dependencies explicit, you can switch to explicit imports and pull inredirectly:import re from uk_bin_collection.uk_bin_collection.common import create_webdriverThen remove the
import *line. This keeps the new behavior intact while resolving the lint warning and making the dependency oncreate_webdriverandreexplicit.uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py (1)
1-4: Consider explicit imports for clarity.The static analysis tool flags that
re,datetime, anddate_formatmay be undefined due to the star import on line 4. While this likely works correctly as a project-wide pattern, explicit imports improve code clarity and make dependencies obvious.uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py (1)
11-12: Consider replacing*import and makingdatetimeimports explicit (to satisfy Ruff F405)Ruff is flagging
datetime(...)here because it’s being pulled in viafrom uk_bin_collection.uk_bin_collection.common import *, which makes the name resolution ambiguous to static analysis. To make this file self‑contained and silence F405, you could switch to explicit imports, e.g.:-from uk_bin_collection.uk_bin_collection.common import * +from datetime import datetime, timedelta +from uk_bin_collection.uk_bin_collection.common import get_dates_every_x_daysThis keeps behaviour the same while avoiding
*imports and clarifying wheredatetime/timedeltacome from.Also applies to: 45-51
uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py (1)
1-1: Consider explicit imports to address static analysis warnings.The star import makes it unclear where
datetimeand other symbols come from, triggering Ruff warnings (F405). While this appears to be a project-wide pattern, explicit imports improve code clarity and help static analysis tools.Consider replacing the star import with explicit imports:
-from uk_bin_collection.uk_bin_collection.common import * +from uk_bin_collection.uk_bin_collection.common import datetime, timedelta, get_dates_every_x_daysuk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py (1)
110-112: Consider refactoring exception handling per static analysis hints.Static analysis suggests abstracting the exception raise to an inner function and avoiding long messages in the exception instantiation. However, the current implementation is clear and functional.
If you want to address the linter hints:
+def _raise_ics_not_found(address: str) -> None: + """Helper to raise ValueError when ICS file cannot be found.""" + raise ValueError(f"Could not find collection ICS file for address: {address}") +Then replace the raise statement:
- raise ValueError( - f"Could not find collection ICS file for address: {user_paon}" - ) + _raise_ics_not_found(user_paon)uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py (1)
25-28: User-Agent header addition is sensible; consider adding a timeoutExplicit
Cookie+ realisticUser-Agentis a good way to keep the council site happy. Since this scraper runs over the network, it would be safer to add a finite timeout so a slow endpoint doesn’t hang the entire run, e.g.:- response = requests.get(URI, headers=headers) + response = requests.get(URI, headers=headers, timeout=10)Tune the timeout to whatever matches your typical end‑to‑end runtime.
Also applies to: 31-31
uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py (1)
22-24: Headers addition is good; add a timeout torequests.getDefining a
headersdict with a browser‑like user agent and passing it intorequests.getshould help avoid the site blocking the scraper, but the call still has no timeout. To avoid hangs on network issues, consider:- response = requests.get(url, headers=headers) + response = requests.get(url, headers=headers, timeout=10)(or another suitable timeout for your environment).
Based on static analysis hints.Also applies to: 47-47
uk_bin_collection/uk_bin_collection/councils/LondonBoroughLambeth.py (1)
45-52: Consider case-insensitive matching for "Recycling" check.The string comparison
"Recycling" in Container["Name"]is case-sensitive. If the API returns "recycling" or "RECYCLING", this check would fail and incorrectly classify it as "refuse".if Container["DisplayPhrase"] == "commercial bin": Bin_Type = ( "recycling" - if "Recycling" in Container["Name"] + if "recycling" in Container["Name"].lower() else "refuse" )uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py (1)
97-120: Minor: Avoid shadowing thetypebuiltin.The loop variable
typeon line 112 shadows Python's built-intype()function. While functionally correct here, it's a common style issue that can cause confusion.- for type in collection_types: - - dict_data = { - "type": type, + for coll_type in collection_types: + dict_data = { + "type": coll_type, "collectionDate": collectiondate, } data_bins["bins"].append(dict_data)uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py (1)
41-43: Add error handling for API response.If the API returns an error or unexpected structure,
soup.find("p")or the JSON parsing could fail with an unclear error.Consider adding validation:
result = soup.find("p").contents[0] + if not result: + raise ValueError("Unexpected API response format") json_data = json.loads(result)["NextCollection"]uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py (2)
71-71: Excessive hardcoded sleep.The 10-second sleep will significantly slow down every request. Consider reducing this or using a more targeted wait condition.
- time.sleep(10) + time.sleep(2) # Allow dropdown options to populateIf the dropdown needs more time to load, consider using an explicit wait for the expected number of options instead.
48-50: Cloudflare bypass approach is fragile.Checking for "Just a moment" in the title is reasonable, but the subsequent 3-second sleep is arbitrary. This may fail intermittently.
Consider adding retry logic or a more robust wait condition if Cloudflare challenges are common.
uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py (2)
1-1: Remove unused import.The
timemodule is imported but never used.-import time - import requests
69-82: Add defensive check for missing jobsField.If the API response doesn't contain
jobsField, this will raise aKeyErrorwith no context.+ if "jobsField" not in bin_collection: + raise ValueError("Unexpected API response: missing jobsField") + # Loop through each collection in bin_collection for collection in bin_collection["jobsField"]:uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py (2)
89-100: Consider using a session for polling requests.Creating a new connection for each poll iteration is inefficient. Using a
requests.Session()would reuse the TCP connection and improve performance during the polling loop.+ session = requests.Session() for attempt in range(1, self.MAX_POLLING_ATTEMPTS + 1): - response = requests.get(url, headers=headers, timeout=10) + response = session.get(url, headers=headers, timeout=10) soup = BeautifulSoup(response.text, features="html.parser")
141-159: Date parsing assumes specific text format.The parsing extracts
date_parts[1](day) anddate_parts[2](month), assuming the format is always "DayName Day Month". If the council changes the format or includes additional text, this will silently fail. Consider adding logging when date parsing fails to aid debugging.uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py (1)
37-43: Potential compatibility issue withJSONDecodeErrorimport.
requests.exceptions.JSONDecodeErrorwas introduced in requests 2.27.0. For broader compatibility, consider catchingjson.JSONDecodeErrordirectly or usingValueErroras a fallback.+from json import JSONDecodeError + ... try: if not response.json().get("auth-session"): raise ValueError("Failed to obtain session cookie") - except requests.exceptions.JSONDecodeError as e: + except JSONDecodeError as e: raise ValueError("Failed to decode session response as JSON") from euk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py (1)
67-76: Robust calendar guard looks good; consider explicitreimport for linting clarityThe narrowed
re.searchscope plus explicitValueErrorif the calendar fragment is missing is a good defensive improvement and will fail fast if the page structure changes.Ruff’s F405 hint about
recoming via thecommonstar import is reasonable; to keep things explicit and linter‑friendly, you can add a direct import at the top:-import time - -import requests +import time +import re + +import requestsI’d keep the current error message despite TRY003; it’s concise and actionable enough here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.github/workflows/behave_pull_request.yml(4 hunks).github/workflows/behave_schedule.yml(4 hunks).github/workflows/bump.yml(1 hunks).github/workflows/codeql-analysis.yml(1 hunks).github/workflows/docker-image.yml(1 hunks).github/workflows/ha_compatibility_test.yml(2 hunks).github/workflows/hacs_validation.yml(1 hunks).github/workflows/lint.yml(1 hunks).github/workflows/release.yml(1 hunks).github/workflows/rollback-release.yml(1 hunks).github/workflows/validate-release-ready.yml(1 hunks).github/workflows/wiki.yml(1 hunks)pyproject.toml(1 hunks)uk_bin_collection/tests/input.json(8 hunks)uk_bin_collection/uk_bin_collection/councils/ArgyllandButeCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py(3 hunks)uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py(3 hunks)uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/LondonBoroughLambeth.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/NorwichCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/SouthLanarkshireCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py(4 hunks)uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/WestOxfordshireDistrictCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py(3 hunks)uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py(3 hunks)uk_bin_collection_api_server/requirements.txt(1 hunks)wiki/Councils.md(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
create_webdriver(321-360)
uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py (6)
uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py (2)
CouncilClass(27-119)parse_data(67-119)uk_bin_collection/uk_bin_collection/councils/RochfordCouncil.py (2)
CouncilClass(9-59)parse_data(16-59)uk_bin_collection/uk_bin_collection/councils/ChorleyCouncil.py (1)
CouncilClass(14-148)uk_bin_collection/uk_bin_collection/councils/KingstonUponThamesCouncil.py (2)
CouncilClass(20-86)parse_data(27-86)uk_bin_collection/uk_bin_collection/councils/EastleighBoroughCouncil.py (2)
CouncilClass(11-89)parse_data(18-89)uk_bin_collection/uk_bin_collection/councils/NorthWestLeicestershire.py (2)
CouncilClass(16-114)parse_data(23-114)
uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (3)
check_postcode(36-49)check_paon(52-64)create_webdriver(321-360)
uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py (2)
uk_bin_collection/uk_bin_collection/common.py (3)
check_paon(52-64)check_postcode(36-49)check_uprn(67-78)uk_bin_collection/uk_bin_collection/get_bin_data.py (1)
AbstractGetBinDataClass(43-146)
uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py (3)
uk_bin_collection/uk_bin_collection/councils/NuneatonBedworthBoroughCouncil.py (1)
get_bin_data(63-945)uk_bin_collection/uk_bin_collection/get_bin_data.py (1)
AbstractGetBinDataClass(43-146)uk_bin_collection/uk_bin_collection/common.py (2)
check_uprn(67-78)check_postcode(36-49)
uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py (1)
custom_components/uk_bin_collection/calendar.py (1)
event(54-63)
uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py (2)
uk_bin_collection/uk_bin_collection/common.py (2)
check_postcode(36-49)check_uprn(67-78)uk_bin_collection/uk_bin_collection/get_bin_data.py (1)
AbstractGetBinDataClass(43-146)
uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py (2)
uk_bin_collection/uk_bin_collection/get_bin_data.py (1)
AbstractGetBinDataClass(43-146)uk_bin_collection/uk_bin_collection/common.py (1)
check_uprn(67-78)
uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
check_postcode(36-49)
uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
check_uprn(67-78)
🪛 Ruff (0.14.7)
uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py
47-47: Probable use of requests call without timeout
(S113)
uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py
40-40: create_webdriver may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py
34-34: Unused method argument: page
(ARG002)
70-70: Avoid specifying long messages outside the exception class
(TRY003)
74-74: Avoid specifying long messages outside the exception class
(TRY003)
99-99: Create your own exception
(TRY002)
99-99: Avoid specifying long messages outside the exception class
(TRY003)
105-105: Create your own exception
(TRY002)
105-105: Avoid specifying long messages outside the exception class
(TRY003)
uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py
68-68: re may be undefined, or defined from star imports
(F405)
69-69: re may be undefined, or defined from star imports
(F405)
72-74: Avoid specifying long messages outside the exception class
(TRY003)
uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py
27-27: check_postcode may be undefined, or defined from star imports
(F405)
28-28: check_paon may be undefined, or defined from star imports
(F405)
38-38: create_webdriver may be undefined, or defined from star imports
(F405)
66-66: Unused lambda argument: d
(ARG005)
83-85: Abstract raise to an inner function
(TRY301)
83-85: Avoid specifying long messages outside the exception class
(TRY003)
102-104: Abstract raise to an inner function
(TRY301)
102-104: Avoid specifying long messages outside the exception class
(TRY003)
126-126: date_format may be undefined, or defined from star imports
(F405)
131-131: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py
36-36: datetime may be undefined, or defined from star imports
(F405)
37-37: datetime may be undefined, or defined from star imports
(F405)
39-39: datetime may be undefined, or defined from star imports
(F405)
40-40: datetime may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py
99-99: Abstract raise to an inner function
(TRY301)
99-99: Avoid specifying long messages outside the exception class
(TRY003)
107-107: datetime may be undefined, or defined from star imports
(F405)
108-108: datetime may be undefined, or defined from star imports
(F405)
124-124: datetime may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py
60-60: re may be undefined, or defined from star imports
(F405)
61-61: datetime may be undefined, or defined from star imports
(F405)
64-64: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
53-53: Avoid specifying long messages outside the exception class
(TRY003)
63-63: Avoid specifying long messages outside the exception class
(TRY003)
66-69: Avoid specifying long messages outside the exception class
(TRY003)
74-74: Avoid specifying long messages outside the exception class
(TRY003)
78-81: Avoid specifying long messages outside the exception class
(TRY003)
101-105: Avoid specifying long messages outside the exception class
(TRY003)
109-113: Avoid specifying long messages outside the exception class
(TRY003)
137-137: Avoid specifying long messages outside the exception class
(TRY003)
140-143: Avoid specifying long messages outside the exception class
(TRY003)
149-149: Avoid specifying long messages outside the exception class
(TRY003)
158-158: Unused method argument: page
(ARG002)
197-199: Avoid specifying long messages outside the exception class
(TRY003)
234-236: Avoid specifying long messages outside the exception class
(TRY003)
uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py
45-45: datetime may be undefined, or defined from star imports
(F405)
46-46: datetime may be undefined, or defined from star imports
(F405)
47-47: datetime may be undefined, or defined from star imports
(F405)
49-49: datetime may be undefined, or defined from star imports
(F405)
50-50: datetime may be undefined, or defined from star imports
(F405)
51-51: datetime may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py
113-113: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py
5-5: from uk_bin_collection.uk_bin_collection.common import * used; unable to detect undefined names
(F403)
17-17: Unused method argument: page
(ARG002)
21-21: check_uprn may be undefined, or defined from star imports
(F405)
22-22: check_postcode may be undefined, or defined from star imports
(F405)
41-41: Probable use of requests call without timeout
(S113)
59-59: Probable use of requests call without timeout
(S113)
77-77: datetime may be undefined, or defined from star imports
(F405)
80-80: date_format may be undefined, or defined from star imports
(F405)
85-85: datetime may be undefined, or defined from star imports
(F405)
85-85: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py
102-102: datetime may be undefined, or defined from star imports
(F405)
105-105: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py
39-39: Avoid specifying long messages outside the exception class
(TRY003)
41-41: Avoid specifying long messages outside the exception class
(TRY003)
67-67: Avoid specifying long messages outside the exception class
(TRY003)
70-70: Avoid specifying long messages outside the exception class
(TRY003)
96-96: Avoid specifying long messages outside the exception class
(TRY003)
114-118: Avoid specifying long messages outside the exception class
(TRY003)
140-140: Avoid specifying long messages outside the exception class
(TRY003)
142-144: Avoid specifying long messages outside the exception class
(TRY003)
178-178: Avoid specifying long messages outside the exception class
(TRY003)
uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py
6-6: from uk_bin_collection.uk_bin_collection.common import * used; unable to detect undefined names
(F403)
18-18: Unused method argument: page
(ARG002)
21-21: check_uprn may be undefined, or defined from star imports
(F405)
32-32: Probable use of requests call without timeout
(S113)
48-48: datetime may be undefined, or defined from star imports
(F405)
54-54: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py
110-112: Abstract raise to an inner function
(TRY301)
110-112: Avoid specifying long messages outside the exception class
(TRY003)
uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py
25-25: check_postcode may be undefined, or defined from star imports
(F405)
29-29: requests may be undefined, or defined from star imports
(F405)
44-44: Abstract raise to an inner function
(TRY301)
44-44: Create your own exception
(TRY002)
44-44: Avoid specifying long messages outside the exception class
(TRY003)
64-64: Abstract raise to an inner function
(TRY301)
64-64: Create your own exception
(TRY002)
64-64: Avoid specifying long messages outside the exception class
(TRY003)
76-76: Abstract raise to an inner function
(TRY301)
76-76: Create your own exception
(TRY002)
76-76: Avoid specifying long messages outside the exception class
(TRY003)
90-90: datetime may be undefined, or defined from star imports
(F405)
93-93: date_format may be undefined, or defined from star imports
(F405)
98-98: datetime may be undefined, or defined from star imports
(F405)
98-98: date_format may be undefined, or defined from star imports
(F405)
101-101: Consider moving this statement to an else block
(TRY300)
uk_bin_collection/uk_bin_collection/councils/SouthLanarkshireCouncil.py
74-74: date_format may be undefined, or defined from star imports
(F405)
82-82: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py
19-19: check_uprn may be undefined, or defined from star imports
(F405)
29-29: requests may be undefined, or defined from star imports
(F405)
⏰ 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). (1)
- GitHub Check: Run Integration Tests (3.12, 1.8.4)
🔇 Additional comments (52)
uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py (2)
39-39: Verify the field name in the API response.The code now extracts data from
BinCollectionInformation. Ensure this field exists in the API response for the newDomesticBinCollections2025onparameter.
24-24: The new API parameterDomesticBinCollections2025onis confirmed active and returns valid bin collection data. The old parameterDomesticBinCollectionsis now deprecated and no longer returns data. The parameter update is correct and necessary for the scraper to function.uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py (1)
44-51: Date roll‑forward keeps schedule logic intactThe updated 2025 base dates for Week 1 and Week 2 still fall on Mondays and preserve the existing 14‑day cadence and relative offsets between refuse, recycling, and glass collections. The control flow and downstream calculations remain consistent with the previous implementation.
uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py (2)
20-34: LGTM! Documentation improvements enhance clarity.The updated docstring provides better context about the dynamic ICS retrieval and parameter usage.
101-113: Add URL resolution for potentially relative ICS hrefs.The
icaleventslibrary does not automatically resolve relative URLs. If the extractedhrefis relative (e.g.,/path/to/calendar.ics), passing it directly toevents()at line 124 will fail. Useurllib.parse.urljointo ensure the URL is absolute:+from urllib.parse import urljoinThen update the extraction logic:
if a: - ics_url = a["href"] + ics_url = urljoin(calendar_url, a["href"]) else: raise ValueError( f"Could not find collection ICS file for address: {user_paon}" )uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py (1)
51-52: LGTM! More robust falsy check.Changing from
item == ""tonot itemcorrectly handles both missing keys (None) and empty strings, preventing potential runtime errors when callingdatetime.fromisoformat(item)on line 54..github/workflows/validate-release-ready.yml (1)
14-14: actions/checkout v6 upgrade is compatible.The upgrade introduces breaking changes to persist-credentials behavior (credentials now written to a separate file under
$RUNNER_TEMPinstead of.git/config) and requires Actions Runner v2.329.0+ for self-hosted runners that use Docker container actions accessing persisted credentials. These changes don't impact standard workflows like this one. The upgrade is safe to proceed..github/workflows/docker-image.yml (1)
25-25: Checkout action bump to v6 looks finePure version bump of
actions/checkoutwith no other workflow changes; consistent with the rest of the PR. Just ensurev6is available in your GitHub Actions environment..github/workflows/release.yml (1)
16-16: Release workflow checkout v6 change is safeOnly the checkout step version is updated; rest of the publish flow is untouched, so behavior should remain identical.
.github/workflows/behave_pull_request.yml (1)
17-17: Consistent migration toactions/checkout@v6across PR workflowAll jobs (setup, unit, parity, integration) now use the same major version of
actions/checkout, which keeps behavior consistent across the pipeline. No other logic changed.Also applies to: 64-64, 95-95, 128-128
uk_bin_collection/uk_bin_collection/councils/ArgyllandButeCouncil.py (1)
1-1: UPRN normalization to 12 digits is a good hardeningCasting
user_uprntostrandzfill(12)before feeding it into the<select>value should make the lookup robust to callers passing non‑padded or integer UPRNs. The addeddatetimeimport aligns with existing usage below.Also applies to: 31-31
.github/workflows/rollback-release.yml (1)
23-23: Rollback workflow checkout bump is straightforwardOnly the checkout action version changed; rollback logic, GH CLI usage, and PyPI steps remain as before.
pyproject.toml (1)
80-85: Version bump automation now also updates API server requirementsAdding
uk_bin_collection_api_server/requirements.txt:uk-bin-collectiontoversion_fileswill keep the API server’s pinned dependency in sync with the library version, which should reduce missed bumps on releases. TOML array syntax looks valid.uk_bin_collection_api_server/requirements.txt (1)
1-3: LGTM!Good consolidation of Connexion extras into a single line, and adding the minimum version constraint for
uk-bin-collectionensures API compatibility.uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py (1)
43-43: LGTM!Padding the UPRN to 12 digits ensures compatibility with the address dropdown's expected value format. This aligns with similar patterns in other council scrapers.
uk_bin_collection/uk_bin_collection/councils/NorwichCityCouncil.py (1)
72-91: LGTM!Good improvements: the extraction now correctly targets
ulelements (fixing what appears to be a typo fromu1), adds defensive length checking, and properly strips whitespace from extracted text.uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py (1)
98-117: LGTM! Cleaner extraction logic.The simplified parsing with direct extraction from
thandtdelements is cleaner. The more specific table selector (class="data-table") reduces fragility.Consider adding defensive checks if the table structure might vary (e.g., empty tbody), though this may be unnecessary if the upstream page structure is stable.
uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py (1)
100-108: LGTM on the new extraction approach.The refactored logic correctly extracts collection dates from the
data-cal-dateattribute and handles multiple collection types per event by splitting on " and ".uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py (4)
19-38: Good documentation addition.The detailed docstring clearly documents parameters, return value, and exceptions. This improves maintainability.
80-96: Resilient CSS selectors using contains-based matching.Using lambda-based class checks with
inoperators makes the parsing more resilient to minor CSS class name changes. This is a solid defensive approach.
98-105: Defensive validation for the collections container.The check prevents silent failures when the page structure changes unexpectedly.
110-134: Robust card parsing with proper null checks.The defensive checks for missing
h3and date elements (lines 112-114, 118-120) prevent crashes on malformed cards. The year rollover logic correctly handles collections spanning the year boundary.uk_bin_collection/uk_bin_collection/councils/WestOxfordshireDistrictCouncil.py (1)
57-63: XPath selector updated for changed UI structure.The XPath change from
dropdown-element-23tocombobox-input-23-1-23aligns with updates to the council's web page. Ensure this has been tested against the live site.uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py (3)
73-86: Clean helper function for address matching.The
_best_optionfunction provides clear, case-insensitive matching logic with good fallback behavior.
100-128: Table parsing with proper validation.The parsing correctly validates the table exists, skips the header row, handles missing data gracefully, and extracts colour from image alt text.
134-142: Proper driver cleanup in finally block.The try/except/finally pattern ensures the WebDriver is always closed, preventing resource leaks.
uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py (1)
84-88: Sorting and return logic looks correct.The bins are properly sorted by collection date before returning.
uk_bin_collection/uk_bin_collection/councils/MertonCouncil.py (1)
68-74: LGTM - UPRN validation is thorough.Good defensive programming with both presence and numeric-only validation for the UPRN parameter.
uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py (1)
43-56: LGTM - Collection parsing logic is sound.The null check on line 47 properly guards against missing elements, and the date parsing correctly formats the output.
uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py (1)
82-95: LGTM - Collection parsing and sorting logic.The extraction of dates and bin types from the list items, along with the sorting by collection date, follows a clean pattern.
wiki/Councils.md (3)
675-685: LGTM - Blackpool documentation added correctly.The new Blackpool council entry follows the established format with appropriate parameters and FindMyAddress reference.
1949-1958: LGTM - Harlow documentation added correctly.The new Harlow council entry is consistent with other UPRN-based council entries.
2557-2564: LGTM - Mid Sussex documentation updated.The removal of
-w(Selenium webdriver) parameter aligns with the code refactor from Selenium to HTTP-based approach.uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py (4)
18-31: Hardcoded API credentials noted - verify this is acceptable.The Basic auth header is extracted from a public mobile app. While this is documented and the approach is pragmatic, be aware that if the council rotates these credentials, the scraper will break. Consider adding a comment about monitoring for credential changes.
83-98: Address matching may miss some edge cases.The paon matching checks if the house number equals the first word of an address line. This could miss cases like "Flat 1, Building Name" or "1A" vs "1". The error messages appropriately guide users to manually find their UPRN, which is a good fallback.
201-231: LGTM - Container processing is robust.The iteration handles missing containers, empty dates, and invalid date formats gracefully by continuing to the next container. The internal
_sort_dateapproach for sorting is clean.
233-247: LGTM - Clean output generation.Raising an error when no valid bins are found provides clear feedback, and the list comprehension cleanly strips the internal sort key.
uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py (4)
1-19: LGTM!Imports and constants are well-organized. The use of named constants for API endpoints and lookup IDs improves maintainability.
22-30: LGTM!The class structure and session initialization are clean. Using
requests.Sessionfor connection pooling and cookie persistence is appropriate for this authentication flow.
45-70: LGTM!The lazy session initialization pattern in
_run_lookupis well-implemented. The debug logging on unexpected response structure is helpful for troubleshooting. The sameJSONDecodeErrornote from the previous comment applies here (line 66).
120-165: LGTM!The
parse_datamethod has good input validation with clear error messages. Supporting both UPRN and postcode+PAON lookup provides flexibility for users. The UTC-aware date for the API payload is appropriate.uk_bin_collection/tests/input.json (8)
232-240: LGTM!BlackpoolCouncil entry is complete with all necessary fields. The UPRN-based approach with FindMyAddress reference follows the established pattern.
957-965: LGTM!FifeCouncil updated to use postcode/house_number with Selenium driver. The wiki_note correctly reflects the new requirements.
1144-1151: LGTM!HarlowCouncil entry is complete and follows the established pattern for UPRN-based lookups.
1279-1286: LGTM!The IsleOfAngleseyCouncil test entry aligns well with the implementation. The
skip_get_url: trueis appropriate since the scraper uses its own session-based API calls rather than the inheritedget_datamethod.
1537-1543: LGTM!MertonCouncil migration to UPRN-based lookup is cleanly implemented with appropriate
skip_get_urlflag.
1733-1740: LGTM!NorthHertfordshireDistrictCouncil updated to use the Cloud9 API endpoint with UPRN. The URL and
uprnfield are consistent.
2056-2061: LGTM!RushmoorCouncil updated to use UPRN-based lookup with appropriate documentation.
1572-1579: LGTM!MidSussexDistrictCouncil entry updated with simplified lookup approach.
uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py (3)
19-32: Docstring now accurately reflects UPRN-driven parsingThe updated docstring clearly describes the UPRN input and the returned
binsstructure, and matches the current implementation (including the fact thatpageis not used for extraction).
56-58: UPRN query parameter change – verify against live endpoint/testsSwitching to
params = {"UPRN": user_uprn}is a sensible simplification, but it changes the exact query string the council endpoint sees. Please double-check this against the live form or existing integration tests to confirm the server still returns the correct calendar for the provided UPRN.
79-93: Result extraction and date sorting logic remain correctThe loop building
bindata["bins"]and the subsequent sort bycollectionDate(parsed as%d/%m/%Y) are consistent with the regex (%m/%d/%Yinput → normalizedDD/MM/YYYYoutput) and look correct.
| r = session.get( | ||
| URI, | ||
| ) | ||
| r.raise_for_status() |
There was a problem hiding this comment.
Add timeout to initial request.
The initial session.get call also needs a timeout for consistency and reliability.
r = session.get(
URI,
+ timeout=30,
)🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py
around lines 32-35, the initial session.get call lacks a timeout which can hang;
add a timeout parameter to that call (e.g., timeout=10) or use the
module/project-wide DEFAULT_TIMEOUT constant if one exists so the request uses
the same timeout policy as other requests in the file, and ensure tests/usage
reflect the chosen timeout value.
| response = requests.get( | ||
| "https://www.rushmoor.gov.uk/Umbraco/Api/BinLookUpWorkAround/Get?", | ||
| params=params, | ||
| headers=headers, | ||
| ) | ||
| # Make a BS4 object | ||
| soup = BeautifulSoup(page.text, features="lxml") | ||
| soup = BeautifulSoup(response.text, features="lxml") |
There was a problem hiding this comment.
Add timeout to HTTP request.
The requests.get call lacks a timeout, which could cause the scraper to hang indefinitely if the server is unresponsive.
Apply this diff:
response = requests.get(
- "https://www.rushmoor.gov.uk/Umbraco/Api/BinLookUpWorkAround/Get?",
+ "https://www.rushmoor.gov.uk/Umbraco/Api/BinLookUpWorkAround/Get",
params=params,
headers=headers,
+ timeout=30,
)Note: The trailing ? in the URL is also unnecessary since params handles query string construction.
🧰 Tools
🪛 Ruff (0.14.7)
29-29: requests may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py around lines
29 to 35, the requests.get call should include a timeout to avoid hanging and
the trailing '?' in the URL is unnecessary; update the call to use the base URL
without the '?' and pass a timeout (e.g. timeout=10) to requests.get (and
optionally keep existing params/headers as-is), and ensure you handle non-200
responses appropriately (e.g. response.raise_for_status() or check status_code)
after the call.
| bluebrownstartDate = datetime(2025, 11, 17) | ||
| greengreystartDate = datetime(2025, 11, 24) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the get_dates_every_x_days function to understand its behavior
rg -n "def get_dates_every_x_days" --type=py -A 20Repository: robbrad/UKBinCollectionData
Length of output: 1933
🏁 Script executed:
# Check the context around lines 36-37 in ThurrockCouncil.py to see how these dates are used
fd "ThurrockCouncil.py" --type f | xargs cat -n | head -100Repository: robbrad/UKBinCollectionData
Length of output: 3656
🏁 Script executed:
# Look for any date filtering logic that might remove past dates
rg -n "filter|datetime\.now\(\)|today\(\)|strptime" uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py -C 3Repository: robbrad/UKBinCollectionData
Length of output: 1022
Update start dates to 2026 to avoid including past collection dates in results.
The hard-coded start dates (November 17 and 24, 2025) are now in the past. Since get_dates_every_x_days generates 28 future dates without filtering past dates, the first ~2 weeks of the returned collection dates will be historical. Update both bluebrownstartDate and greengreystartDate to November 2026 to ensure results contain only current and future collection schedules.
🧰 Tools
🪛 Ruff (0.14.7)
36-36: datetime may be undefined, or defined from star imports
(F405)
37-37: datetime may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py around lines
36-37 the hard-coded start dates are set to datetime(2025, 11, 17) and
datetime(2025, 11, 24), which are now in the past; update both
bluebrownstartDate and greengreystartDate to use November 2026 (datetime(2026,
11, 17) and datetime(2026, 11, 24)) so get_dates_every_x_days produces only
current/future collection dates. Ensure the datetime imports are correct and run
tests to verify schedules begin in 2026.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ncil.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…Council.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ouncil.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Changed to use dedicated API to retrieve collection data
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @robbrad. * #1754 (comment) The following files were modified: * `uk_bin_collection/uk_bin_collection/councils/ArgyllandButeCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ArmaghBanbridgeCraigavonCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/BirminghamCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/BlackpoolCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/EdinburghCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/FarehamBoroughCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/FifeCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/HaltonBoroughCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/HarlowCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/IsleOfAngleseyCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/KingsLynnandWestNorfolkBC.py` * `uk_bin_collection/uk_bin_collection/councils/LondonBoroughLambeth.py` * `uk_bin_collection/uk_bin_collection/councils/MidSussexDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NorthHertfordshireDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NorthumberlandCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NorwichCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/RushmoorCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthGloucestershireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthHollandDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthLanarkshireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ThurrockCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WestOxfordshireDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WiltshireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WinchesterCityCouncil.py`
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.