Allow injecting a requests.Session to keep logins from piling up

[proscar87]

Follows up on the session-persistence idea from #160, which you asked for there.

Why

Every GrowattApi builds its own fresh requests.Session, so nothing is shared or persisted. As you described, in the HA integration login() 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 that requests also 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.

The hook is installed once per session: a second GrowattApi on the same session sees the first one's hook and doesn't add another. The first version of this PR appended one per instance, so three instances left three hooks and every response was checked three times.

What a caller should know before sharing a session:

• The hook runs on every response that goes through that session, including requests that have nothing to do with Growatt. A non-2xx answer from another service raises HTTPError from raise_for_status.
• With Raise GrowattRateLimitError when a response body carries Growatt's 507 code #160 merged as well, the same hook also reads every response body, so a foreign HTTP 200 whose JSON happens to carry success: false, msg: "507" would raise GrowattRateLimitError. I checked this with both heads merged (no conflicts): one hook after three instances, and that body raises GrowattRateLimitError.
• So the safe pattern is a session dedicated to Growatt, shared only between GrowattApi instances.

Validation

Nine tests covering: default session still created, injected session used as-is, two instances sharing one, restored cookies surviving, pre-existing hooks kept, the hook installed once when three instances share a session, each response checked once with three instances, bare-callable hooks normalised, and the user-agent applied to the injected session. Eight of the nine fail on master (without the parameter). The two hook-count tests also fail on this PR's first commit (assert 4 == 2, assert 3 == 1). Full suite: 10 passed (master: 1). ruff 0.15.16 and mypy --ignore-missing-imports growattServer/ are clean, as in CI. Merged with #160's current head: 20 passed, ruff and mypy clean.

Not validated: I did not test against Growatt's server, so I never checked that restored cookies are actually accepted there, or for how long. The tests only show that the cookies stay on the injected session; whether Growatt honours them in place of a fresh login() is unmeasured.

🤖 Generated with Claude Code

[johanzander]

Thanks, sharing a session and restoring cookies is a good way to reduce re-logins and 507 lockouts. Appending to existing hooks rather than replacing them is the right call, and the tests cover it well.

Main point: headers.update overwrites the User-Agent on an injected session even when no agent_identifier was passed, so a caller's own User-Agent is silently replaced. With several instances on one session, the last one wins. Consider only setting it when the caller passed an agent_identifier.

Minor:

• De-duplicating by __qualname__ would also match any unrelated hook with the same qualname. A module-level hook function or a marker attribute, compared by identity, would be sturdier.
• The hook doesn't catch the 507 lockout, which comes back as HTTP 200 with msg: "507" in the body. That's fine, but the docstring could say that skipping login() while the persisted cookies are still valid is up to the caller, and that the session now raises on HTTP errors for any request.

[proscar87]

Thanks @johanzander, all three addressed in 5bb15b8:

• User-Agent: an injected session's User-Agent is only changed when the caller passes agent_identifier or add_random_user_id (asking for a random suffix is asking for the header to change). A session the library creates itself still gets the default, as before.
• Hook de-duplication: _raise_for_status is now a module-level function, matched by identity. A test puts an unrelated hook with the same name on the session and checks both end up installed.
• Docstring: it now says that skipping login() while restored cookies are still valid is up to the caller, that the session raises on HTTP errors for every request made through it, and that the 507 lockout (HTTP 200 with msg: "507") is not caught by that hook.

Four new tests; each fails against the change it guards. ruff check ./growattServer (0.15.16, as in CI) and mypy pass.

[proscar87]

Merged master to resolve the conflict with #160 (9d31cfb). Since #160 put the 507 check in the response hook, the module-level _raise_for_status now runs _raise_if_rate_limited first, so an injected session gets it too, and a new test covers that. It also means my earlier note was wrong: the hook does now catch the 507 lockout, and the docstring says so. 25 tests pass; ruff (./growattServer, 0.15.16) and mypy are clean.

[johanzander]

Thanks for addressing the points. Checked the latest head (9d31cfb): the User-Agent handling, identity-based hook matching and docstring all look good, and the 507 check from #160 now applies to injected sessions too. mypy and ruff pass in CI, and all 25 tests pass locally.

[johanzander]

@indykoning - want to have a look before merging?

[indykoning]

Looks good and i appreciate the addition of the tests!