feat: serve an appversions.json index file to clients via IMAP metadata - #1038
Conversation
missytake
left a comment
missytake
left a comment
There was a problem hiding this comment.
No big deal to merge this I think. three small nitpicks:
| self.appversions_path = Path( | ||
| params.pop("appversions_path", "/usr/local/lib/chatmaild/appversions.json") | ||
| ) |
There was a problem hiding this comment.
Not sure whether this needs to be a configurable value tbh. I think hardcoding this path would be fine. Not so important though, as operators don't really need to find out about it.
There was a problem hiding this comment.
i removed it completely.
| ) | ||
|
|
||
| deployer.put_file( | ||
| src=get_reporoot().joinpath("APPVERSIONS.json").open("rb"), |
There was a problem hiding this comment.
Hm, I think the file should rather be in cmdeploy/src/cmdeploy than reporoot - if we link to it from CONTRIBUTING.md, client devs will be able to find it I think. But that's just cleanliness preference I guess.
There was a problem hiding this comment.
agreed, discussed with @r10s and shifted it to chatmaild python package into a chatmaild/defaults/appversions.json (no need to capitalize as is only common at repo root) where we can also move the chatmail.ini.
j4n
left a comment
j4n
left a comment
There was a problem hiding this comment.
Looks good from what I can understand, however, the 24h refresh suggestion from #1037 seems worth mentioning as being not addressed yet; relay releases were indeed few and far between compared to client releases in the last months.
| @@ -0,0 +1,15 @@ | |||
| { | |||
There was a problem hiding this comment.
Should this file maybe have a timestamp?
There was a problem hiding this comment.
it wouldn't help much currently with freshness of app versions to users. Freshness pipe from relay-repo -> relay-deployment -> core reading new metadata -> UI showing "update available" rather depends on core currently. it's not clear what checks/automatizations we want to do, so let's stick with the minimum data for now.
GitHub Actions
Inactive
This is designed to help implement self-updating APKs (and later other clients), see counterpart chatmail/core#8557
| logging.exception(f"failed to read {path}") | ||
| return None | ||
| # the dict protocol is line-based, keep the value single-line | ||
| return json.dumps(data, separators=(",", ":")) |
There was a problem hiding this comment.
the separators are just to minimize the generated string. they would be ", " and ": " otherwise :)
r10s
changed the title
5 participants
fixes #1037
This is designed to help implement self-updating APKs (and later other clients), see core counterpart chatmail/core#8557