feat: add rate limit tracking and public API#148
feat: add rate limit tracking and public API#148banter240 wants to merge 3 commits intoerwindouna:mainfrom
Conversation
erwindouna
left a comment
There was a problem hiding this comment.
Thanks for contributing, here an initial review. :)
e6e3c02 to
b46987b
Compare
Essential improvements for production use. Rate Limit Support: - Store response headers from API calls - Parse RateLimit headers per IETF draft spec - rate_limit property returning (limit, remaining) tuple - last_headers property for debugging Public API (enables extensions): - session property (get aiohttp ClientSession) - home_id property (get home ID) - access_token property (get OAuth token) - refresh_auth() public method (refresh token) Model Fixes: - ZoneState null nextTimeBlock handling (API sometimes returns null) Breaking Changes: None (only additions and bug fixes) All changes are backward compatible.
b46987b to
2b4e258
Compare
erwindouna
left a comment
There was a problem hiding this comment.
Making solid progress. tadoasync is heavily tested. Would you mind including/updating tests, that way we can ensure we're releasing a solid package. :)
| @property | ||
| def session(self) -> ClientSession: | ||
| """Return the aiohttp session.""" | ||
| return self._ensure_session() | ||
|
|
||
| @property | ||
| def home_id(self) -> int | None: | ||
| """Return the home ID.""" | ||
| return self._home_id | ||
|
|
||
| @property | ||
| def access_token(self) -> str | None: | ||
| """Return the OAuth access token.""" | ||
| return self._access_token | ||
|
|
||
| async def refresh_auth(self) -> None: | ||
| """Refresh the OAuth token.""" | ||
| await self._refresh_auth() |
There was a problem hiding this comment.
Not be too nitpicky, but we never exposed these before. I suppose this is from your other work? The refresh auth is something I might have doubts on, since the business logic should be that it checks per request if a refresh is needed.
There was a problem hiding this comment.
Yeah, you're right, this is from my TadoHijack integration.
I need these for TadoX API support. The new Tado X hardware uses a completely separate API at hops.tado.com that's not covered by tadoasync's existing methods. I have to make direct HTTP requests to that API while reusing tadoasync's session and auth:
In TadoXApi class
await self._tado._refresh_auth() # Refresh before our external request
headers = {"Authorization": f"Bearer {self._tado._access_token}"}
url = f"https://hops.tado.com/homes/{self._tado._home_id}/rooms"
async with self._tado._ensure_session().request(...) as response:
return await response.json()
Since we're making requests to an external API (outside of tadoasync's _request() flow), we need manual access to the session, auth token, and refresh logic. Currently we're accessing _refresh_auth(), _access_token, _home_id, and _ensure_session() as private attributes, which is why we'd like to see them public.
The refresh_auth() method is needed because we're bypassing tadoasync's normal request flow entirely - we need to manually refresh before our own external API calls.
There was a problem hiding this comment.
I see, that supports the hybrid solution, right?
There was a problem hiding this comment.
Yes, exactly. Once we manage to integrate native Tado X support directly into tadoasync, we can refactor this and maybe make them private again. But for now, while I have to handle the Tado X API externally alongside tadoasync (the hybrid approach), it's much safer to have these exposed publicly rather than relying on private _ members.
There was a problem hiding this comment.
Makes sense, I can accept that change.
Co-authored-by: Erwin Douna <e.douna@gmail.com>
|
So I think, the only thing left are updating the tests. :) |
Essential improvements for production use:
Rate Limit Support (NEW):
Public API (NEW - enables extensions):
Model Fixes:
Breaking Changes: None (only additions and one critical bug fix)
All changes are backward compatible. Existing code works unchanged.
Enables Home Assistant integrations to: