Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added automatic app version detection #50

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sockless-coding
Copy link
Contributor

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

@lostfields
Copy link
Owner

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.

@sockless-coding
Copy link
Contributor Author

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).

@LudovicRousseau
Copy link
Contributor

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}

@sockless-coding
Copy link
Contributor Author

@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.

@lostfields
Copy link
Owner

lostfields commented Jul 15, 2021

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
Copy link
Owner

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

@sockless-coding
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@lostfields
Copy link
Owner

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?

@jkaberg
Copy link

jkaberg commented Nov 17, 2021

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

@Th0masDB Th0masDB mentioned this pull request Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants