Expose the date/time that the v2 access token expires

[zxdavb]

The v2 api uses access tokens. 0.2.5 seems to have a 60 min timeout, whilst 0.2.6 has a 30 min timeout!

In any case, the code utilising the evohome-client needs to use _login() to obtain another access token before the existing token expires. The PR exposes the date/time that the access token expires via client.access_token_expires. It also moves the initialization of client.access_token from __init__() to _login().

This change is much simpler than trying to deal with refresh tokens.

This PR should not affect existing users of this library.

[watchforstock]

Thanks for this. I've actually used what you've done as inspiration for automatically refreshing the credentials. As you've noted above, I've just logged in again (albeit without refreshing the site data) rather than using the refresh token. You can see it here on a branch: https://github.com/watchforstock/evohome-client/tree/auto-renew

Would you be able to test this (I've done a basic test here and it's renewed ok after 1/2 an hour). If it seems to be working, I can merge it into the main branch as it should have no (negative) impact on other users

[zxdavb]

No problem - let me play with it for a while & I'll get back to you.

[zxdavb]

First impressions: I think that the following lines should not be in __init__().

self.access_token = None
self.locations = []

I'd move self.access_token = None to before the request.post() in _basic_login(self). That way, if _basic_login() fails for whatever reason, then the access token be None (rather than a stale access token. The same would apply for self.access_token_expires.

I 'moved' self.locations = [] to just before the for loop in installation(self) in PR #44 , but forgot to remove it from __init__() as I'd intended.

[zxdavb]

I'd also put a little breathing space into the token timeout:

def headers(self):
    if datetime.now() > self.access_token_expires:
        self._basic_login()
    return self._headers

This is to cope with the not insignificant propagation delay between client and server:

if datetime.now() > self.access_token_expires - timedelta(seconds = 15):

You see I use 15 seconds, but maybe 30 might be better? You can see from my debugging logs that I've already lost 5 seconds...

2018-06-21 21:41:49 INFO (SyncWorker_4) [custom_components.evohome] Access token expires: 2018-06-21 22:11:45.998307

[watchforstock]

All good points! I've pushed an update to the branch incorporating the above. I've set the breathing space at 30s so perhaps we can see how that goes

[zxdavb]

OK, it survived over an hour (i.e. 2x re-login()s) of my code.

Since my code includes a work-around for the token timeout, I'm going to run it overnight, but this time without any work-around...

I like what you did with the self.headers() - clever!

[watchforstock]

Thanks, although I realised afterwards I could have put a @property on a function so I didn't need to change all of the references to a function call. In fact, I may do this in a bit just in case anyone is making use of the header attribute (unlikely, but not impossible)

Fingers crossed it works overnight...

[zxdavb]

If nothing else, I've learnt a lot of git today!

On Thu, 21 Jun 2018 at 22:56 Andrew Stock notifications@github.com wrote:

Thanks, although I realised afterwards I could have put a @property on a function so I didn't need to change all of the references to a function call. In fact, I may do this in a bit just in case anyone is making use of the header attribute (unlikely, but not impossible)

Fingers crossed it works overnight...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/watchforstock/evohome-client/pull/46#issuecomment-399256097, or mute the thread https://github.com/notifications/unsubscribe-auth/AGeFCKGoCzUM-jsBC32CaoXSk0znA1pbks5t_BaNgaJpZM4Uyha6 .

[zxdavb]

OK, my git skills weren't as good as I'd hoped!

I'm having an issue this end, but I've had to add some logging to your code for me to see what's going on. I'm running the test again (it failed last night, but may be my fault).

In any case, I was going to submit another PR with a r.raise_for_status() after each request. This is really needed as the honeywell servers can be unreliable (status 500/501), but also for a 429 (request limit exceeded):

r = requests.get('https://tccna.honeywell.com/WebAPI/emea/api/v1/location/%s/status?includeTemperatureControlSystems=True' % self.locationId, headers=self.client.headers)

if r.status_code != requests.codes.ok:
    r.raise_for_status()

Would you accept a PR like that?

[watchforstock]

That suggestion for a PR sounds sensible, others have highlighted errors at some times and as you point out, it could be how the library is being used rather than the library itself so makes sense to raise the error to be handled at an application level

[zxdavb]

OK, leave it to me!

I had a lot of errors last night, but no real way to tell what went on without status code (401, 429, or 50x?). I'll run it again, and see what eventuates...

[watchforstock]

Thanks for all your contributions. I've now merged the auto-renew branch into master so you'll want to switch to that. I'll close this PR as we've incorporated the changes elsewhere.

[zxdavb]

Thanks.