Update Citybikes component with third-party library and fields #151009
Conversation
eskerda
force-pushed
the
ref-citybikes
branch
2 times, most recently
from
922f0f1 to
e1738b7
Compare
| radius, UnitOfLength.FEET, UnitOfLength.METERS | ||
| ) | ||
|
|
||
| client = CitybikesClient(user_agent=HA_USER_AGENT, timeout=REQUEST_TIMEOUT) |
There was a problem hiding this comment.
why do we need to add the user agent?
There was a problem hiding this comment.
The previous code using the aio_httpclient helper included HomeAssistant/... on the user agent, which was certainly useful for myself to realize this component existed. I added it again with the intention of keeping track of usage and maybe failures.
LMK if we should go with the default UA on python-citybikes, which would be python-citybikes/<version>.
There was a problem hiding this comment.
Wait you work on/for city bikes?
There was a problem hiding this comment.
I do maintain the Citybikes project and the API that this component uses :) But it's a common name, so not to be confused with any other Citi Bike.
|
|
||
| async def async_refresh(self, now=None): | ||
| """Refresh the state of the network.""" | ||
| try: |
There was a problem hiding this comment.
Only have things in the try block that can raise
There was a problem hiding this comment.
I have moved some of the logic outside of the try block now. That said, unfortunately I am missing most of the intent of the original code and I am not sure what now does in this context. Let me know if we should do a bigger refactor here with a coordinator.
There was a problem hiding this comment.
Good! I am happy to bring the component out from legacy in other PRs.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
eskerda
force-pushed
the
ref-citybikes
branch
from
e1738b7 to
aad594c
Compare
|
@joostlek Is there anything left on my side for approving this PR? I see there are failing codecov checks. Not sure if I need to add any test? Thanks! |
3 participants
Proposed change
According to #96764 and #85279 the main blocker on updating the citybikes component was not relying on a third party library.
This PR updates the component to use
python-citybikesto fetch data and adds an extraebikesfield.Ideally I would like to overhaul the component to bring it up in quality, but I figured this would be a good start without breaking changes.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: