Allow injecting a requests.Session to keep logins from piling up - #162
Open
proscar87 wants to merge 1 commit into
Open
Allow injecting a requests.Session to keep logins from piling up#162proscar87 wants to merge 1 commit into
proscar87 wants to merge 1 commit into
Conversation
Requested by @johanzander in indykoning#160: every GrowattApi instance builds its own fresh Session, so nothing is shared or persisted. In the Home Assistant integration login() is called from the config flow, from setup, and from the coordinator on every update cycle -- which is the login frequency that drives accounts into the ~24h 507 lockout that indykoning#160 only reports. A constructor-injected `session` lets a consumer share one Session across API instances *and* restore persisted cookies at startup. The library takes on no storage responsibility: where the cookies live stays the caller's business. The raise_for_status hook is now appended rather than assigned. A shared Session may already carry hooks and replacing `session.hooks` outright would drop them silently; a bare callable is normalised to a list, which requests permits. Six of the seven tests fail without the parameter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows up on the session-persistence idea from #160, which you asked for there.
Why
Every
GrowattApibuilds its own freshrequests.Session, so nothing is shared or persisted. As you described, in the HA integrationlogin()is called from the config flow, from setup, and from the coordinator on every update cycle — and that login frequency is what drives accounts into the ~24h lockout. #160 only reports the lockout; this is the part that can prevent it.The change
A constructor-injected
session: requests.Session | None = None, the shape you preferred. It lets a consumer share one session across API instances and restore persisted cookies at startup. The library stores nothing itself — where the cookies live stays the caller's business.Default behaviour is unchanged: with no argument it creates its own session exactly as before.
One thing worth reviewing
self.session.hooks = {"response": [_raise_for_status]}assigned the hooks dict, which is fine for a session this library owns but destructive for one it was handed — a caller sharing a session across libraries would have their own hooks silently dropped. It now appends, and normalises the bare-callable form thatrequestsalso permits.If you'd rather this library not touch a caller-supplied session's hooks at all, say so and I'll gate it — but then a shared session loses
raise_for_status, which the rest of the code relies on, so it would need a different error path.Validation
Seven tests covering: default session still created, injected session used as-is, two instances sharing one, restored cookies surviving, pre-existing hooks kept, bare-callable hooks normalised, and the user-agent applied to the injected session. Six of the seven fail without the parameter. Full suite green.
🤖 Generated with Claude Code