search

tronikos / opower

A Python library for getting historical and forecasted usage/cost from utilities that use opower.com such as PG&E
Apache License 2.0
65 stars 59 forks source link

Ambiguous: "Re-login to make sure code handles already logged in sessions." #66

Closed BrianHenryIE closed 9 months ago

BrianHenryIE commented 10 months ago

In demo.py, the login is performed twice.

https://github.com/tronikos/opower/blob/1900209b9d862d8edb6c30d8a472153169525eda/src/demo.py#L86-L88

Is this to test that it remembers the existing login, or is the correct behaviour to discard cookies and force a refreshed login?

I spent time at the weekend working on #54 – andylittle/opower-smud/pull/1 – where I'm returning early if the cookies are already set.

tronikos commented 10 months ago

Under some unknown circumstances Home Assistant calls the login twice within the same second. That was causing issues for some implementation of utilities. If you can remember the existing login it would be better but if it's easier to discard cookies you can go with that approach.

tronikos commented 10 months ago

Looking at your implementation, don't you need to check if the login has expired to not return early? What will happen when Home Assistant refreshes data 12 hours later and the cookie exists but the login has expired?

Are you planning on including these changes to this library? Could you reuse async_auth_saml in helpers.py?

BrianHenryIE commented 10 months ago

check if the login has expired

I don't know TBH! I would hope the cookies would expire when the auth has expired, but I will add a check. I'm mostly a PHP developer so haven't worked with long running processes in a while!

Are you planning on including these changes to this library?

Yes. I opened the PR there because it needs SMUD customers to test it. Once one or two people have confirmed it works for them, I'll open a PR here. I just noticed today it will need a rebase first.

Could you reuse async_auth_saml in helpers.py?

I'll check that out. Thank you.