libremesh / lime-packages

LibreMesh packages configuring OpenWrt for wireless mesh networking
https://libremesh.org/
GNU Affero General Public License v3.0
274 stars 94 forks source link

Eupgrade is_new_version_available always will return false #1069

Closed selankon closed 3 months ago

selankon commented 5 months ago

I noticed that if I use the RPC call:

ubus call eupgrade is_new_version_available '{}'

It will always return false (actually something like {"status":"ok"}). This is because of the RPC function will always ask for cached latest versions:

local function is_new_version_available(msg)
    local cached_only = true
    local ret = {}
    if eupgrade.is_enabled() then
        local latest_data = eupgrade.is_new_version_available(cached_only)
        if latest_data then
            ret['version'] = latest_data.version
            ret["release_info_url"] = latest_data['release-info-url']
        end
    end
    ret.status = 'ok'
    utils.printJson(ret)
end

https://github.com/libremesh/lime-packages/blob/master/packages/eupgrade/files/usr/libexec/rpcd/eupgrade#L19-L30

The relevant code here is local cached_only = true, which mean, that on the function that check if new version is available, it check the cahced latest version, and i none, return false. https://github.com/libremesh/lime-packages/blob/master/packages/eupgrade/files/usr/lib/lua/eupgrade.lua#L68-L70

I didn't find any place on the packages where actually look the non cached eupgrade.is_new_version_available, but maybe I am missing something. How can i force to download new information?

@spiccinini is this really an error?

I'll propose to set the local cached_only = true as default, but with the ability to call the is_new_version_available with a parameter to override it:

ubus call eupgrade is_new_version_available '{ "cached_only" : false }'
spiccinini commented 5 months ago

Yes, it is indeed a bug. I dont remember exactly why now, I believe that when I wrote the code the implementation was to not cache by default. But I think that the we rush to make a release and the lime app was not providing the cached_only property and it was asking for this data on app load so it was very slow to start so I hardcoded the value in eupgrade.lua and forgot about it. And as upgrades we were doing in communities were rolling slowly nee releases it was "ok" to load new firmware info only once per boot.

I think it is better to have cached_only default to false. Or to change the scripts in bin/

selankon commented 5 months ago

I created a PR. What do you think to add a function to get the cached only new version available? Could be useful? Probably the refactor to adapt limeapp will be to much bigger compared to the functionality added.

I mean, if we would like to take advantage to call cache only or not, we have to refactor a lot of parts maybe.

ilario commented 3 months ago

The fix originally included in #1070 has been merged in #1101. I will close this issue but feel free to re-open it if there is more to discuss.