November release#1701
Conversation
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 5. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v5) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
fix: #1685 - New URL
fix: #1685 - New URL
fix: #1688 - BREAKING CHANGE
fix: #1382 - Removed the need for Selenium
Removing the requirement for Selenium and moving to UPRN
…ow.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…cil.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>
…ndling; normalise dd/MM/YYYY
…s; robust iframe/cookie handling
Docstrings generation was requested by @Jam3zs. * #1678 (comment) The following files were modified: * `uk_bin_collection/uk_bin_collection/councils/TendringDistrictCouncil.py`
fix(tendring): read 'Next collection' column; harden cookie/iframe handling; normalise dd/MM/YYYY
fix: Council Fix Pack - November 2025
fix: Herefordshire Council
…ns/upload-artifact-5 chore: bump actions/upload-artifact from 4 to 5
Enhance HTTP requests with retry logic and headers
📝 Add docstrings to `fix/tendring-next-collection`
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new Dumfries and Galloway council parser, updates test data and docs, and refactors many council parsers—moving several from HTML scraping to API flows, improving HTTP headers/retries, enhancing HTML parsing robustness, and introducing encrypted payload handling for Newport. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Client/CLI
participant Session as HTTP Session
participant API as Council API
participant Parser as Local Parser
rect rgb(230,230,240)
note over CLI,Parser: Legacy scraping (Selenium/BeautifulSoup)
CLI->>Parser: launch webdriver / GET page
Parser->>API: request HTML page
API-->>Parser: HTML
Parser->>Parser: parse DOM, extract bins
Parser-->>CLI: return bins
end
rect rgb(220,245,220)
note over CLI,Session: API-driven flow (Hounslow/Middlesbrough/Rochdale)
CLI->>Session: POST auth / obtain token
Session->>API: auth request
API-->>Session: token/session
Session->>API: POST data request (token + payload)
API-->>Session: JSON response
Session->>Parser: parse JSON -> bins
Parser-->>CLI: return bins
end
rect rgb(245,230,230)
note over CLI,Session: Encrypted transport (Newport)
CLI->>CLI: AES-CBC encrypt payload (hex)
CLI->>Session: POST encrypted hex
Session->>API: forward encrypted request
API-->>Session: encrypted hex response
Session->>CLI: deliver hex
CLI->>CLI: decrypt -> parse JSON -> bins
CLI-->>Parser: return bins
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (15)
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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @robbrad. * #1701 (comment) The following files were modified: * `uk_bin_collection/uk_bin_collection/councils/BostonBoroughCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/BrightonandHoveCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/DerbyCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/DumfriesandGallowayCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/HartDistrictCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/HerefordshireCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/LondonBoroughHarrow.py` * `uk_bin_collection/uk_bin_collection/councils/LondonBoroughHounslow.py` * `uk_bin_collection/uk_bin_collection/councils/LondonBoroughSutton.py` * `uk_bin_collection/uk_bin_collection/councils/MiddlesbroughCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/RochdaleCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py` * `uk_bin_collection/uk_bin_collection/councils/WokinghamBoroughCouncil.py`
❌ 2 Tests Failed:
View the top 1 failed test(s) by shortest run time
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
📝 Add docstrings to `November_release`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py (1)
25-40: Consider making browser version dynamic or adding a comment.The headers include hardcoded Chrome version "141" in both
user-agentandsec-ch-ua. While this works currently, these values will become outdated over time. Consider either making them dynamic or adding a comment explaining they're static for compatibility reasons.uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py (2)
85-94: Add response structure validation.The code assumes
decoded_binshas the expected structure without validation. If the API response format changes or is malformed, this will raiseKeyError.Add validation:
decoded_bins = self.decode_response(output) + + if not isinstance(decoded_bins, dict) or "collectionDay" not in decoded_bins: + raise ValueError("Invalid API response structure: missing 'collectionDay'") + + if not isinstance(decoded_bins["collectionDay"], list): + raise ValueError("Invalid API response: 'collectionDay' must be a list") + data: dict[str, list[dict[str, str]]] = {} data["bins"] = list( map( lambda a: { "type": a["binType"], "collectionDate": a["collectionDay"].replace("-", "/"), }, decoded_bins["collectionDay"], ) )
96-100: Consider using proper logging instead of print statements.The exception handler uses
print()for error output. The codebase appears to use a logger (as seen inget_bin_data.py).+import logging + +_LOGGER = logging.getLogger(__name__) + except Exception as e: - # Here you can log the exception if needed - print(f"An error occurred: {e}") - # Optionally, re-raise the exception if you want it to propagate + _LOGGER.error(f"Failed to fetch Newport bin collection data: {e}") raiseuk_bin_collection/uk_bin_collection/councils/BrightonandHoveCityCouncil.py (1)
32-40: Keepkwargs["url"]as an override
Hard-coding the enviroservices endpoint means anyone relying on a config-drivenurl(tests, alternate environments, or future domain shifts) can no longer point the scraper elsewhere without editing code. Please keep reading the URL fromkwargsand only fall back to this constant if it is missing so we retain that flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/ha_compatibility_test.yml(1 hunks)uk_bin_collection/tests/input.json(7 hunks)uk_bin_collection/uk_bin_collection/councils/BostonBoroughCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/BrightonandHoveCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/DerbyCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/DumfriesandGallowayCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/HartDistrictCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/HerefordshireCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/LondonBoroughHarrow.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/LondonBoroughHounslow.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/LondonBoroughSutton.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/MiddlesbroughCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/RochdaleCouncil.py(2 hunks)uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/TendringDistrictCouncil.py(1 hunks)uk_bin_collection/uk_bin_collection/councils/WokinghamBoroughCouncil.py(2 hunks)wiki/Councils.md(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
uk_bin_collection/uk_bin_collection/councils/BrightonandHoveCityCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
create_webdriver(321-360)
uk_bin_collection/uk_bin_collection/councils/BostonBoroughCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
create_webdriver(321-360)
uk_bin_collection/uk_bin_collection/councils/MiddlesbroughCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
check_paon(52-64)
uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py (2)
uk_bin_collection/uk_bin_collection/common.py (1)
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/TendringDistrictCouncil.py (2)
uk_bin_collection/uk_bin_collection/common.py (3)
check_postcode(36-49)check_uprn(67-78)create_webdriver(321-360)uk_bin_collection/uk_bin_collection/get_bin_data.py (1)
AbstractGetBinDataClass(43-146)
uk_bin_collection/uk_bin_collection/councils/DumfriesandGallowayCouncil.py (3)
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)custom_components/uk_bin_collection/calendar.py (1)
event(54-63)
uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py (1)
uk_bin_collection/uk_bin_collection/common.py (1)
create_webdriver(321-360)
🪛 Gitleaks (8.28.0)
uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py
[high] 13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.14.3)
uk_bin_collection/uk_bin_collection/councils/BrightonandHoveCityCouncil.py
33-33: Local variable uprn is assigned to but never used
Remove assignment to unused variable uprn
(F841)
38-38: create_webdriver may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py
46-46: Probable use of requests call without timeout
(S113)
uk_bin_collection/uk_bin_collection/councils/BostonBoroughCouncil.py
39-39: create_webdriver may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/RochdaleCouncil.py
58-58: Prefer TypeError exception for invalid type
(TRY004)
58-58: Avoid specifying long messages outside the exception class
(TRY003)
74-74: datetime may be undefined, or defined from star imports
(F405)
77-77: datetime may be undefined, or defined from star imports
(F405)
77-77: timedelta may be undefined, or defined from star imports
(F405)
103-103: Prefer TypeError exception for invalid type
(TRY004)
103-103: Avoid specifying long messages outside the exception class
(TRY003)
105-105: Loop control variable key not used within loop body
Rename unused key to _key
(B007)
108-108: datetime may be undefined, or defined from star imports
(F405)
110-110: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/MiddlesbroughCouncil.py
7-7: from uk_bin_collection.uk_bin_collection.common import * used; unable to detect undefined names
(F403)
12-12: Unused method argument: page
(ARG002)
18-18: check_paon may be undefined, or defined from star imports
(F405)
30-30: Probable use of requests call without timeout
(S113)
39-39: f-string without any placeholders
Remove extraneous f prefix
(F541)
52-52: Probable use of requests call without timeout
(S113)
104-104: date_format may be undefined, or defined from star imports
(F405)
116-116: Consider moving this statement to an else block
(TRY300)
uk_bin_collection/uk_bin_collection/councils/HerefordshireCouncil.py
35-35: Avoid specifying long messages outside the exception class
(TRY003)
62-62: datetime may be undefined, or defined from star imports
(F405)
62-62: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/LondonBoroughSutton.py
54-54: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
62-64: Avoid specifying long messages outside the exception class
(TRY003)
uk_bin_collection/uk_bin_collection/councils/LondonBoroughHounslow.py
56-56: Prefer TypeError exception for invalid type
(TRY004)
56-56: Avoid specifying long messages outside the exception class
(TRY003)
69-69: datetime may be undefined, or defined from star imports
(F405)
72-72: datetime may be undefined, or defined from star imports
(F405)
72-72: timedelta may be undefined, or defined from star imports
(F405)
98-98: Prefer TypeError exception for invalid type
(TRY004)
98-98: Avoid specifying long messages outside the exception class
(TRY003)
100-100: json may be undefined, or defined from star imports
(F405)
105-105: datetime may be undefined, or defined from star imports
(F405)
107-107: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/DumfriesandGallowayCouncil.py
12-12: from uk_bin_collection.uk_bin_collection.common import * used; unable to detect undefined names
(F403)
17-17: Unused method argument: page
(ARG002)
23-23: check_uprn may be undefined, or defined from star imports
(F405)
42-42: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/ChelmsfordCityCouncil.py
12-12: from uk_bin_collection.uk_bin_collection.common import * used; unable to detect undefined names
(F403)
17-17: Unused method argument: page
(ARG002)
28-28: create_webdriver may be undefined, or defined from star imports
(F405)
45-47: try-except-pass detected, consider logging the exception
(S110)
45-45: Do not catch blind exception: Exception
(BLE001)
45-45: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
84-86: Abstract raise to an inner function
(TRY301)
84-86: Avoid specifying long messages outside the exception class
(TRY003)
103-103: date_format may be undefined, or defined from star imports
(F405)
uk_bin_collection/uk_bin_collection/councils/WokinghamBoroughCouncil.py
97-97: datetime may be undefined, or defined from star imports
(F405)
102-102: date_format may be undefined, or defined from star imports
(F405)
🔇 Additional comments (6)
.github/workflows/ha_compatibility_test.yml (1)
184-184: ✅ Safe action version upgrade.The v5 update supports Node v24.x and is not a breaking change per se. The
overwriteparameter (default 'false') is supported and will delete a matching artifact before uploading a new one if true. Since each artifact gets a unique name via the matrix variable (ha-log-${{ matrix.ha_version }}), theoverwrite: truesetting is harmless here, though technically unnecessary. The upgrade is fully compatible with the existing workflow.uk_bin_collection/uk_bin_collection/councils/HartDistrictCouncil.py (1)
46-50: LGTM!The logic correctly handles multiple bin types per row by splitting on "&" and creating separate dictionary entries with a shared collection date. The
strip()call ensures clean bin type values.uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py (4)
32-47: LGTM: Encryption implementation is correct.The AES-CBC encryption with PKCS7 padding is properly implemented using the cryptography library.
49-64: LGTM: Decryption implementation is correct.The AES-CBC decryption with PKCS7 unpadding correctly reverses the encryption process.
68-72: LGTM: UPRN validation and input construction.The UPRN validation and NewportInput construction with fixed literal values is correct. The
check_uprnfunction will raise an exception for invalid UPRNs, preventing further processing.
76-84: Add timeout, response validation, and consider Content-Type header.The HTTP request lacks resilience features used throughout the codebase:
- Missing timeout — all other council implementations specify timeout (10–120s); this request can hang indefinitely
- No response validation — response.text is accessed without checking status_code; error responses may parse incorrectly
- No Content-Type header — encoded body data should specify content type; verify with API documentation whether this is required
Suggested fix:
- session = requests.Session() - response = session.post( + response = requests.post( "https://iweb.itouchvision.com/portal/itouchvision/kmbd/collectionDay", data=encoded_input, + timeout=30, ) + response.raise_for_status() output = response.textNote: Determine the correct Content-Type from API documentation (e.g.,
application/octet-streamif encoded_input is binary). Removing Session is also recommended since it's only used for one request.
| if "bin" not in bin_type.lower(): | ||
| continue |
There was a problem hiding this comment.
Don’t drop sack/caddy collections
For sack-served properties this guard drops legitimate entries, so parse_data returns no general‑rubbish schedule. Example: UPRN 10024616751 renders “General rubbish - black sacks”; because it lacks the substring “bin”, this branch skips it and the bins array is incomplete. That’s a functional regression for any address served by sacks (or future caddy/bag variants). Please broaden the filter rather than hard-coding “bin”. One way:
- if "bin" not in bin_type.lower():
+ lowered_bin_type = bin_type.lower()
+ if not any(keyword in lowered_bin_type for keyword in ("bin", "sack", "caddy")):
continue| headers = {"User-Agent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64)"} | ||
| response = requests.get(URI, headers=headers, timeout=30) |
There was a problem hiding this comment.
🧩 Analysis chain
Good improvement, but verify error handling for request exceptions.
Adding the User-Agent header and timeout parameter improves robustness and follows best practices. However, the timeout parameter introduces requests.exceptions.Timeout as a potential exception. Ensure proper error handling exists for timeout and other request exceptions (e.g., ConnectionError, HTTPError).
Run the following script to check if error handling exists elsewhere in the codebase for similar request patterns:
🏁 Script executed:
#!/bin/bash
# Description: Check for error handling patterns around requests.get calls in council parsers
# Search for requests.get with try-except blocks in council files
rg -n -B2 -A5 'requests\.get' uk_bin_collection/uk_bin_collection/councils/ | rg -A5 'try:'Length of output: 6424
I need to inspect the LondonBoroughHarrow.py file directly to verify whether error handling exists around the requests.get call.
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Examine the context around requests.get in LondonBoroughHarrow.py
rg -n -B5 -A10 'requests\.get' uk_bin_collection/uk_bin_collection/councils/LondonBoroughHarrow.py
Length of output: 107
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Search for LondonBoroughHarrow file in the repository
fd -i 'londonborough.*harrow' . --type f
Length of output: 69
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Examine the context around requests.get in LondonBoroughHarrow.py
rg -n -B8 -A8 'requests\.get' ./uk_bin_collection/uk_bin_collection/councils/LondonBoroughHarrow.py
Length of output: 689
Add error handling for request timeout and exceptions.
The timeout parameter introduces requests.exceptions.Timeout that will not be caught. Additionally, response.json() (line 31) and nested dict access (line 34) lack error handling and will crash on network failures, JSON parsing errors, or missing keys. Wrap the request and JSON parsing in a try-except block, following patterns used in other council parsers (e.g., RotherhamCouncil, MidlothianCouncil).
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/LondonBoroughHarrow.py around
lines 27-28, wrap the requests.get call, response.json() and subsequent dict
accesses in a try-except block similar to RotherhamCouncil/MidlothianCouncil:
catch requests.exceptions.RequestException (including Timeout),
json.JSONDecodeError and KeyError (or TypeError) and handle them by logging the
error and returning an empty list or appropriate default so the parser doesn't
crash; ensure you still set headers and timeout, and return early on exception.
| if not isinstance(rows_data, dict): | ||
| raise ValueError("Invalid data returned from API") | ||
|
|
||
| collections = json.loads(rows_data["jobsJSON"]) |
There was a problem hiding this comment.
Import json before calling json.loads.
json is never imported in this module, so the first call to json.loads(...) will raise a NameError, halting the collection flow. Add the missing import near the top of the file.
import time
import requests
+import jsonCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.3)
100-100: json may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/LondonBoroughHounslow.py around
line 100, the code calls json.loads(rows_data["jobsJSON"]) but the module never
imports the json module; add "import json" near the top of the file (with other
imports) so json.loads is defined and avoid a NameError at runtime.
| if r.status_code == 429: | ||
| # manual respect if upstream Retry didn’t catch (e.g., no header) | ||
| retry_after = int(r.headers.get("Retry-After", "10")) | ||
| time.sleep(min(retry_after, 60)) | ||
| r.raise_for_status() | ||
| poll += 1 |
There was a problem hiding this comment.
Allow the poll loop to survive 429 responses
Right after you detect a 429 you sleep, but then r.raise_for_status() runs anyway and throws HTTPError, so the loop never retries and Sutton stays broken under rate limiting. The raw int() on Retry-After will also crash if the header is a HTTP-date or decimal value. Please short-circuit the loop on 429 and make the header parsing defensive.
- if r.status_code == 429:
- # manual respect if upstream Retry didn’t catch (e.g., no header)
- retry_after = int(r.headers.get("Retry-After", "10"))
- time.sleep(min(retry_after, 60))
- r.raise_for_status()
- poll += 1
+ if r.status_code == 429:
+ # manual respect if upstream Retry didn't catch (e.g., no header)
+ try:
+ retry_after = int(r.headers.get("Retry-After", "10"))
+ except (TypeError, ValueError):
+ retry_after = 10
+ time.sleep(min(retry_after, 60))
+ poll += 1
+ continue
+ r.raise_for_status()
+ poll += 1🧰 Tools
🪛 Ruff (0.14.3)
54-54: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
| key_hex = "F57E76482EE3DC3336495DEDEEF3962671B054FE353E815145E29C5689F72FEC" | ||
| iv_hex = "2CBF4FC35C69B82362D393A4F0B9971A" |
There was a problem hiding this comment.
Critical: Remove hardcoded cryptographic keys.
Hardcoding AES encryption keys and IVs in source code is a severe security vulnerability. These credentials are now permanently exposed in version control history and accessible to anyone with repository access.
Move these to secure configuration:
-key_hex = "F57E76482EE3DC3336495DEDEEF3962671B054FE353E815145E29C5689F72FEC"
-iv_hex = "2CBF4FC35C69B82362D393A4F0B9971A"
+import os
+
+key_hex = os.environ.get("NEWPORT_API_KEY")
+iv_hex = os.environ.get("NEWPORT_API_IV")
+
+if not key_hex or not iv_hex:
+ raise ValueError("NEWPORT_API_KEY and NEWPORT_API_IV environment variables must be set")Alternatively, use a secure secrets management solution. After moving the keys, rotate them with the API provider if possible.
📝 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.
| key_hex = "F57E76482EE3DC3336495DEDEEF3962671B054FE353E815145E29C5689F72FEC" | |
| iv_hex = "2CBF4FC35C69B82362D393A4F0B9971A" | |
| import os | |
| key_hex = os.environ.get("NEWPORT_API_KEY") | |
| iv_hex = os.environ.get("NEWPORT_API_IV") | |
| if not key_hex or not iv_hex: | |
| raise ValueError("NEWPORT_API_KEY and NEWPORT_API_IV environment variables must be set") |
🧰 Tools
🪛 Gitleaks (8.28.0)
[high] 13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/NewportCityCouncil.py around
lines 13-14 the AES key and IV are hardcoded which is a critical security risk;
remove these literals and instead load the key and IV from secure configuration
(e.g., environment variables or a secrets manager), fail fast with a clear error
if they are missing, and ensure the application never logs or prints the secret
values; update any deployment/config docs to provide the secrets securely and
rotate the exposed keys with the API provider after deployment.
| params = { | ||
| "UPRN": {user_uprn}, | ||
| } |
There was a problem hiding this comment.
Critical: Fix the params dictionary syntax.
Line 43 uses set literal syntax {user_uprn}, which creates a set containing the UPRN value instead of passing the value directly. This will cause the API request to fail or produce incorrect query parameters.
Apply this diff to fix the syntax:
params = {
- "UPRN": {user_uprn},
+ "UPRN": user_uprn,
}📝 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.
| params = { | |
| "UPRN": {user_uprn}, | |
| } | |
| params = { | |
| "UPRN": user_uprn, | |
| } |
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py around
lines 42 to 44, the params dictionary currently uses set literal syntax ("UPRN":
{user_uprn}) which creates a set instead of passing the UPRN value; replace the
set literal with the direct value so the params dict is constructed as "UPRN":
user_uprn (ensuring user_uprn is the expected string/int type for the API).
| r = requests.get( | ||
| "https://www.southampton.gov.uk/whereilive/waste-calendar", | ||
| params=params, | ||
| headers=headers, | ||
| ) |
There was a problem hiding this comment.
Add a timeout to prevent indefinite hangs.
The request lacks a timeout parameter, which could cause the application to hang indefinitely if the server doesn't respond. This is a reliability concern.
Apply this diff to add a timeout:
r = requests.get(
"https://www.southampton.gov.uk/whereilive/waste-calendar",
params=params,
headers=headers,
+ timeout=30,
)📝 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.
| r = requests.get( | |
| "https://www.southampton.gov.uk/whereilive/waste-calendar", | |
| params=params, | |
| headers=headers, | |
| ) | |
| r = requests.get( | |
| "https://www.southampton.gov.uk/whereilive/waste-calendar", | |
| params=params, | |
| headers=headers, | |
| timeout=30, | |
| ) |
🧰 Tools
🪛 Ruff (0.14.3)
46-46: Probable use of requests call without timeout
(S113)
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/SouthamptonCityCouncil.py around
lines 46 to 50, the requests.get call lacks a timeout which can cause indefinite
hangs; update the call to include a timeout parameter (e.g. timeout=10 or a
constant defined near the top) so the request fails fast on no response, and
ensure any surrounding exception handling (requests.exceptions.Timeout) is left
or added to handle timeout errors appropriately.
| bin_data["bins"].sort( | ||
| key=lambda x: datetime.strptime(x.get("collectionDate"), "%d/%m/%Y") | ||
| key=lambda x: datetime.strptime(x["collectionDate"], "%d/%m/%Y") | ||
| ) |
There was a problem hiding this comment.
Sort with the configured date format
You serialise dates with parsed.strftime(date_format) but re-parse them with a hard-coded "%d/%m/%Y" when sorting. If date_format ever changes (user override or future library update) this will crash. Reuse date_format in the key instead of the literal.
🤖 Prompt for AI Agents
In uk_bin_collection/uk_bin_collection/councils/TendringDistrictCouncil.py
around lines 171 to 173, the sort key reparses collectionDate using the
hard-coded "%d/%m/%Y" while dates are serialized with
parsed.strftime(date_format); update the sort to reuse the same date_format
variable (e.g. datetime.strptime(x["collectionDate"], date_format) or
self.date_format if that's the instance attribute) so parsing matches formatting
and will not break if date_format is changed or overridden.
| dt_collection_date = datetime.strptime( | ||
| collection_date.text.strip().split(" ")[1], source_date_format | ||
| ) | ||
| dict_data = { | ||
| "type": waste_type.text.strip().split("(")[0].strip(), | ||
| "collectionDate": dt_collection_date.strftime(date_format), | ||
| } |
There was a problem hiding this comment.
Guard against single-token collection dates
collection_date.text.strip().split(" ")[1] will throw when the span renders only the bare date string (e.g. "14/11/2025") or contains double spaces, which happens for some Wokingham cards. That crashes the whole scrape. Please parse defensively by taking the last non-empty token (and validating it) before feeding it to datetime.strptime.
- dt_collection_date = datetime.strptime(
- collection_date.text.strip().split(" ")[1], source_date_format
- )
+ date_text = collection_date.text.strip()
+ parts = date_text.split()
+ if not parts:
+ raise ValueError(f"Unexpected date format: {date_text!r}")
+ date_token = parts[-1].rstrip(",")
+ dt_collection_date = datetime.strptime(
+ date_token, source_date_format
+ )📝 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.
| dt_collection_date = datetime.strptime( | |
| collection_date.text.strip().split(" ")[1], source_date_format | |
| ) | |
| dict_data = { | |
| "type": waste_type.text.strip().split("(")[0].strip(), | |
| "collectionDate": dt_collection_date.strftime(date_format), | |
| } | |
| date_text = collection_date.text.strip() | |
| parts = date_text.split() | |
| if not parts: | |
| raise ValueError(f"Unexpected date format: {date_text!r}") | |
| date_token = parts[-1].rstrip(",") | |
| dt_collection_date = datetime.strptime( | |
| date_token, source_date_format | |
| ) | |
| dict_data = { | |
| "type": waste_type.text.strip().split("(")[0].strip(), | |
| "collectionDate": dt_collection_date.strftime(date_format), | |
| } |
🧰 Tools
🪛 Ruff (0.14.3)
97-97: datetime may be undefined, or defined from star imports
(F405)
102-102: date_format may be undefined, or defined from star imports
(F405)
5 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation