Conversation
📝 WalkthroughWalkthroughModified NewhamCouncil.py to support food waste bin collection. Changes include disabling SSL verification, adding food waste bin type to recognized collection types, parsing food waste bin data from the page structure, and adjusting the date parsing format from day-first to month-first format. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@uk_bin_collection/uk_bin_collection/councils/NewhamCouncil.py`:
- Around line 47-58: The parsing loop for sections (variables: header,
bin_type_element, array_expected_types, date) assumes a rigid DOM and can raise
AttributeError when chaining item.find_next("p", {"class":
"card-text"}).find_next("mark").next_sibling; modify the loop to defensively
handle missing nodes by either (a) checking each intermediate value is not None
before accessing its children (verify header, bin_p = item.find_next("p",
{"class":"card-text"}), mark = bin_p.find_next("mark"), and mark.next_sibling)
and only then assign date, or (b) wrap the inner parsing in a try/except
AttributeError that logs/continues on failure; ensure the check for bin_type in
array_expected_types remains and that malformed cards are skipped without
raising.
🧹 Nitpick comments (2)
uk_bin_collection/uk_bin_collection/councils/NewhamCouncil.py (2)
1-1: SuppressInsecureRequestWarningsince SSL verification is intentionally disabled.
urllib3is imported but unused. Sinceverify=Falsewill emit anInsecureRequestWarningon every request, you likely intended to suppress it. Also consider adding a code comment explaining why verification is disabled (incomplete certificate chain) so future maintainers don't silently remove it.🛡️ Proposed fix
import urllib3 +urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) from bs4 import BeautifulSoupAlso applies to: 22-22
24-24: Pre-existing:soup.prettifyis a no-op without parentheses.This is a method reference, not a call. It should be
soup.prettify()if the intent is to prettify, or removed entirely since the result isn't used.
| for item in sections: | ||
| header = item.find("div", {"class": "card-header"}) | ||
| bin_type_element = header.find_next("b") | ||
| if bin_type_element is not None: | ||
| bin_type = bin_type_element.text | ||
| array_expected_types = ["Domestic", "Recycling"] | ||
| array_expected_types = ["Domestic", "Recycling", "Food Waste"] | ||
| if bin_type in array_expected_types: | ||
| date = ( | ||
| item.find_next("p", {"class": "card-text"}) | ||
| .find_next("mark") | ||
| .next_sibling.strip() | ||
| ) |
There was a problem hiding this comment.
Chained attribute access can raise AttributeError if the food waste card has a different DOM structure.
header, bin_type_element, and the find_next("p")...find_next("mark").next_sibling chain all assume a specific HTML structure. If the food waste card layout differs even slightly (e.g., missing <mark> tag), this will throw an unhandled AttributeError. This was a pre-existing risk for domestic/recycling but the new food waste type increases the surface area.
Consider wrapping the inner parsing in a try/except or adding None guards.
🤖 Prompt for AI Agents
In `@uk_bin_collection/uk_bin_collection/councils/NewhamCouncil.py` around lines
47 - 58, The parsing loop for sections (variables: header, bin_type_element,
array_expected_types, date) assumes a rigid DOM and can raise AttributeError
when chaining item.find_next("p", {"class":
"card-text"}).find_next("mark").next_sibling; modify the loop to defensively
handle missing nodes by either (a) checking each intermediate value is not None
before accessing its children (verify header, bin_p = item.find_next("p",
{"class":"card-text"}), mark = bin_p.find_next("mark"), and mark.next_sibling)
and only then assign date, or (b) wrap the inner parsing in a try/except
AttributeError that logs/continues on failure; ensure the check for bin_type in
array_expected_types remains and that malformed cards are skipped without
raising.
1 participant
This PR contains 2 fixes and I've also added one type of bin collection (food waste)
Fixes:
verify=FalseFailed setup, will retry: Unexpected error: HTTPSConnectionPool(host='bincollection.newham.gov.uk', port=443): Max retries exceeded with url: /Details/Index/XXXXXXXX (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1032)')))Features:
Summary by CodeRabbit
New Features
Bug Fixes