python-panasonic-comfort-cloud icon indicating copy to clipboard operation
python-panasonic-comfort-cloud copied to clipboard

Added automatic app version detection

Open sockless-coding opened this issue 4 years ago • 10 comments

Added support to fetch the latest published app version from the Apple App Store

sockless-coding avatar Jul 14 '21 10:07 sockless-coding

This is a nice one, but I prefer to cache the file version from apple like we are doing with the token. Lets say we send in a file location for appversion instead of auto, if we got a file location we lookup the version and store it, the lookup is done once a day (looking at file date). If the file ins't provided (may use a default?) we use the constant.

May you do that instead? I don't like pulling data from third party more than necessary.

lostfields avatar Jul 15 '21 08:07 lostfields

The data is only pulled on initialize, how often do you expect the library to be reinitialized? If the version should be cached to disk, the token cache needs to be updated as well to combine both values in one file. Where I use this, initialization is called less than once per week (Home Assistant Component).

sockless-coding avatar Jul 15 '21 13:07 sockless-coding

In my case I use the library in a cron job that is executed every 30 minutes. So my script would ask Apple API 384 times per week.

The best would be to use Apple service (and cache the result) only if the Panasonic server responds with the error:

pcomfortcloud.session.ResponseError: Invalid response, status code: 401 - Data: {"message":"New version app has been published","code":4106}

LudovicRousseau avatar Jul 15 '21 13:07 LudovicRousseau

@LudovicRousseau waiting for the error is not a good option, it looks like they have a 3-week grace period after releasing a new version before blocking the old version.

sockless-coding avatar Jul 15 '21 14:07 sockless-coding

In my case I use the library in a cron job that is executed every 30 minutes. So my script would ask Apple API 384 times per week.

The best would be to use Apple service (and cache the result) only if the Panasonic server responds with the error:

pcomfortcloud.session.ResponseError: Invalid response, status code: 401 - Data: {"message":"New version app has been published","code":4106}

Thumbs up, it's better doing the version check in the login routine instead of in the init, and better yet, only check it when login returns status 40X

lostfields avatar Jul 15 '21 14:07 lostfields

The data is only pulled on initialize, how often do you expect the library to be reinitialized? If the version should be cached to disk, the token cache needs to be updated as well to combine both values in one file. Where I use this, initialization is called less than once per week (Home Assistant Component).

Hmm, I don't know how it's used, but when using it command line it would do this for every command issued

lostfields avatar Jul 15 '21 14:07 lostfields

The data is only pulled on initialize, how often do you expect the library to be reinitialized? If the version should be cached to disk, the token cache needs to be updated as well to combine both values in one file. Where I use this, initialization is called less than once per week (Home Assistant Component).

Hmm, I don't know how it's used, but when using it command line it would do this for every command issued

If it's used in a py file called in say a cron job it will be initialized for each execution, so for that scenario I can see the point of caching it. Though I doubt it would make much of a difference, a cold request takes ~250ms and once they cached it, it's 5ms.

sockless-coding avatar Jul 15 '21 14:07 sockless-coding

I've refactored the token storage and consolidated it with version information into a settings class that stores the information as json. Version information has a TTL of 5 days. TODO maybe add exception handling to the panasonic requests and call _updateAppVersion() if we get the Invalid response, status code: 401 - Data: {"message":"New version app has been published","code":4106} response.

sockless-coding avatar Jul 15 '21 19:07 sockless-coding

sorry for the delay, I see now that you have implemented a few changes and this is starting to look good! Since all users have to authenticate again when updating to the new settings file instead of a token file I have to review it properly.

Have you thought about a migration process, where you update the token file the new format?

lostfields avatar Oct 25 '21 07:10 lostfields

Just a thought - how about fetching the new version when the we actually get rejected by PCC (eg on http error) and cache that until next version bump (hence new http error)? That'd result in a lot less requests, and only on a need to basis

jkaberg avatar Nov 17 '21 07:11 jkaberg