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

WIP: Implement a method of retrieving the package update time #155

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

piggz
Copy link
Contributor

@piggz piggz commented Mar 3, 2023

The package update time is based on the last update of the source pacakge, not the binary package. To do this, I have a server application, currently hosted on piggz.co.uk:8081 which uses the OBS API to build a JSON model, relating projects, packages, repositories and binaries.

To find the package mtime, you iterate the model, searching for the project, then find the repository matching the current device, then look for the package in the binaries. WHen the pacakge is found from the binary, get the mtime from the source package list.

Currently I dont know how to get the repo name from SSU, therfore the arch is hardcoded as aarch64.

Included some simple update to the QML to show the update time, nothing more complicated such as a section of "recently udpated apps"

Server code will be posted later for comment, it could certainly be improved (currently a single large cache), and Im happy for others to take this on if the idea is sound.

The package update time is based on the last update of the _source_
pacakge, not the binary package.  To do this, I have a server
application, currently hosted on piggz.co.uk:8081 which uses the OBS API
to build a JSON model, relating projects, packages, repositories and
binaries.

To find the package mtime, you iterate the model, searching for the
project, then find the repository matching the current device, then look
for the package in the binaries.  WHen the pacakge is found from the
binary, get the mtime from the source package list.

Currently I dont know how to get the repo name from SSU, therfore the
arch is hardcoded as aarch64.

Included some simple update to the QML to show the update time, nothing
more complicated such as a section of "recently udpated apps"

Server code will be posted later for comment, it could certainly be
improved (currently a single large cache), and Im happy for others to
take this on if the idea is sound.
@piggz piggz requested review from rinigus and Olf0 March 3, 2023 21:36
@piggz piggz added enhancement New feature or request help wanted Extra attention is needed labels Mar 3, 2023
@rinigus
Copy link
Contributor

rinigus commented Mar 4, 2023

I would suggest to make mtime as a property of ChumPackage. This would make sense as it is a package property.

Next, I suggest to update the list of packages in Chum or make separate singleton for it. ChumPackagesModel is not a good choice as it is used for different package listing sorting and presentation. That's the one we will expand to make Recent list as well. Look how chumModel is created in PackagesListPage. Would be bad to update the list every time the user moves to a new page ...

When moved JSON list handling to Chum, we can drop ChumPackagesModel.busy and use Chum.busy as it is done already.

In terms of http://piggz.co.uk:8081/ - would be way better to load data only for active repository. Right now it is rather large download (5MB) and took 43 seconds over here. But that could be done later as well. I guess some REST API or similar is needed. Would be good to also have a timestamp file/facility showing when the latest list was generated to avoid loading it if it is not needed. ... Hmm, looking at this file - we probably can just keep mtime section in JSON. No need to have all those packages listed in "repositories".

Currently I dont know how to get the repo name from SSU, therfore the arch is hardcoded as aarch64.

Don't follow, sorry. Note that older SFOS releases don't have aarch64

BTW, I accidently looked into https://repology.org/ . While they have Suse support (and probably via OBS), they don't list modified times in their API, unfortunately. Otherwise we could have used that for a backend...

@Olf0
Copy link
Collaborator

Olf0 commented Mar 4, 2023

Currently I dont know how to get the repo name from SSU, therfore the arch is hardcoded as aarch64.

ssu lr | fgrep sailfishos-chum
Should be easy to determine the corresponding call to libssu at https://github.com/sailfishos/ssu

My thoughts:

  • IMO an external cache is not a good idea: Someone has to run and maintain it. If something happens (illness, car accident etc.) to that someone (currently you @piggz), the whole thing collapses.
    IMO there is enough compute power and bandwidth locally available to perform all data extraction from the SailfishOS-OBS on the device.
  • I do understand the issue, which is minor from my point of view. But the efforts to address it seem to be huge. As I will not contribute code for this, I must leave it up to you to judge if these efforts are inappropriate / out of proportion. What exacerbates this aspect is …
  • … Jolla's trajectory and their handling of SailfishOS: After some consideration, I am sure SailfishOS is deliberately not maintained sustainably and this is not going to change unless a new, big corporate customer pays significantly for it. I.e. one which replaces Rostelecom's millions put in into SFOS. Mind that in the past 7 years Rostelecom was the only significant customer, so the likeliness for that to happen is close to zero. In short: SailfishOS is left dying slowly.

@piggz
Copy link
Contributor Author

piggz commented Mar 4, 2023

Currently I dont know how to get the repo name from SSU, therfore the arch is hardcoded as aarch64.

ssu lr | fgrep sailfishos-chum Should be easy to determine the corresponding call to libssu at https://github.com/sailfishos/ssu

My thoughts:

  • IMO an external cache is not a good idea: Someone has to run and maintain it. If something happens (illness, car accident etc.) to that someone (currently you @piggz), the whole thing collapses.

I do not propose to keep the server on piggz.co.uk, that is just for WIP/demo, it would obviously be better somewhere else, and am open to ideas for this.....

IMO there is enough compute power and bandwidth locally available to perform all data extraction from the SailfishOS-OBS on the device.

There are 2 problems here, 1, OBS API requires credentials, atm, the server app is using my credentials to build the cache.
And 2, it takes close to an hour to actually build the cache with all the calls, so you probably dont want to do this on device :) (yes, it would be cut down by not having to check all version/archs, but its still a slow process)

  • I do understand the issue, which is minor from my point of view. But the efforts to address it seem to be huge. As I will not contribute code for this, I must leave it up to you to judge if these efforts are inappropriate / out of proportion. What exacerbates this aspect is …

  • … Jolla's trajectory and their handling of SailfishOS: After some consideration, I am sure SailfishOS is deliberately not maintained sustainably and this is not going to change unless a new, big corporate customer pays significantly for it. I.e. one which replaces Rostelecom's millions put in into SFOS. Mind that in the past 7 years Rostelecom was the only significant customer, so the likeliness for that to happen is close to zero. In short: SailfishOS is left dying slowly.

@rinigus
Copy link
Contributor

rinigus commented Mar 5, 2023

I think having some kind of extra service is the best way for now. It could be on @piggz domain or some cloud storage. Probably we would need some PC running check once in a while and updating that JSON. As for dependence on some person, yes, that's what comes with SFOS in general.

Re general concerns, I'll reply in the other thread.

Load the cache as the first step in refreshing the repository details
and implement the mtime parameter as a property of the package like all
other properties.
@piggz
Copy link
Contributor Author

piggz commented Mar 5, 2023

@rinigus Ive implemented all your comments. The cache is entirely optional, if cache loading fails, then you are in no worse off situation. I still need to implement the architecture detection to create the correct repository name for searching.

Next step will be to post the server code and get comments on that.

@piggz
Copy link
Contributor Author

piggz commented Mar 5, 2023

Server app is here
https://github.com/sailfishos-open/chum-package-cache

Obvious improvements would be to generate separate caches for different releases, using GET params to get the required version, therefore making the download smaller, though at the moment it is nice and simple, and doesnt require many NPM modules, relying on only GET and POST

@rinigus
Copy link
Contributor

rinigus commented Mar 5, 2023

chum-package-cache has just README, no code :)

@piggz
Copy link
Contributor Author

piggz commented Mar 5, 2023

it does now :)

BusyIndicator {
size: BusyIndicatorSize.Large
anchors.centerIn: parent
running: chumModel.busy == true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed - whole BusyIndicator - as we have busy state in Chum

@@ -78,7 +85,7 @@ Page {
}

onClicked: pageStack.push(Qt.resolvedUrl("../pages/PackagePage.qml"), {
pkg: Chum.package(model.packageId)
pkg: Chum.package(model.packageId),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pkg: Chum.package(model.packageId),
pkg: Chum.package(model.packageId)

#include <QJsonObject>
#include <QJsonArray>

static char* apiUrl = "http://piggz.co.uk:8081";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to add that as MACRO and set it in CMakeLists.txt

m_chumPackageCache = QJsonDocument::fromJson(reply->readAll());
qDebug() << "Received chum package cache";
}
m_busy = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should stay on busy=true as SSU gets loaded as well. As it should be called after network access manager is finished, state busy==true is expected and we don't need to change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, aren't we expected to call reply->deleteLater()?


QDateTime Chum::findPackageMTime(const QString &rpm) const
{
qDebug() << Q_FUNC_INFO << rpm << Chum::instance()->repoName() << Chum::instance()->repoVersion();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use Chum::instance if we have it in this?

Comment on lines +49 to +50
QString repoName() { return m_ssu.repoName();}
QString repoVersion() { return m_manualVersion.isEmpty() ? m_ssu.deviceVersion() : m_manualVersion; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess those are not needed anymore

@@ -261,6 +264,12 @@ void ChumPackage::setPackagerLogin(const QString &login) {
SET_IF_EMPTY(m_packager_login, PackagePackagerRole, login);
}

void ChumPackage::setPackageMTime(const QDateTime &mtime)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not called anymore

@@ -119,6 +122,7 @@ class ChumPackage : public QObject {
void setForksCount(int count);
void setIssuesCount(int count);
void setPackagerLogin(const QString &login);
void setPackageMTime(const QDateTime &mtime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed

@@ -49,6 +49,7 @@ class ChumPackagesModel
void filterUpdatesOnlyChanged();
void searchChanged();
void showCategoryChanged();
void busyChanged();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

@@ -25,6 +26,9 @@ Ssu::Ssu(QObject *parent) :
QDBusConnection::systemBus(),
parent )
{
QDBusReply<QString> version = call(QStringLiteral("release"), false);
m_device_version = version;
qDebug() << "Device version:" << m_device_version;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can override repo version. We should get real URL and use that, as described above

@Olf0
Copy link
Collaborator

Olf0 commented Apr 6, 2023

Off topic: Honestly (and frankly, as usual), IMO fixing the bugs which severely affect basic functionality of the SailfishOS:Chum GUI app should be prioritised higher than new features, specifically technically complex ones, which hence are complicated to implement.

Specifically I consider the bugs #184 and #165 / #103 as pretty bad, because both describe a specific core-function of the GUI app as fundamentally broken. Plus bug #186 appears to be a very "how hanging fruit" (should be trivial to address for someone QML savvy).

Furthermore feature request #86, plus likely also #105 and #126 appear to be easy to implement for a much larger gain that displaying the correct package update time.

But as usual, as we all are doing this in our spare time, for a good part it is fine to prioritise things to their fun factor; to a certain extent, because IMO developers / maintainers should (but not "must") assume a little responsibility for their software.

@rinigus
Copy link
Contributor

rinigus commented Apr 7, 2023

Yes, spring cleaning should be done. And I agree, #184 is rather annoying one. Would prioritize that. Will try to check others as well.

@Olf0
Copy link
Collaborator

Olf0 commented Apr 16, 2023

My thoughts:

  • IMO an external cache is not a good idea: Someone has to run and maintain it. If something happens (illness, car accident etc.) to that someone (currently you @piggz), the whole thing collapses.

I do not propose to keep the server on piggz.co.uk, that is just for WIP/demo, it would obviously be better somewhere else, and am open to ideas for this.....

Oh, neither the DNS name nor you as a person was something I intended to address.
What I meant was:

  • An(other) external service in addition to the ones required by the current design: the SailfishOS-OBS (specifically this OBS instance due to depending on Jolla's DoD-repositories, currently) and a source code hosting service (which may differ for each package).
  • More single points of failure
    • Technically: What happens if this service is not available?
      This might and should be alleviated by the client side implementation: If the service is not reachable, the client shall fall back to its current behaviour.
    • Staff / persons in charge to administer this service: A single person is the worst case here.

AFAIU this does not need to be a permanently running service: Its data gathering is rather cyclical and its output simply must be accessible by clients, i.e. downloadable via HTTPS.
Then a GitHub app or even a GH action may be sufficient: They can be triggered time based (basically alike a cron job), run on a preconfigured VM image (Ubuntu, plus MacOS and Windows) in user space (but sudo is working), install packages locally from the regular package repos (sudo apt get foo), use anything in accessible GH repos via git clone / git checkout (I suggest to create a separate repo for this), can push stuff to the web-site <name>.github.io or any writeable repo etc. In short: Only permanently running services are forbidden.

This would allow for using the regular access privileges at GitHub in order to avoid a "single person" single point of failure and the infrastructure is guaranteed to be kept nearly 100% available by others (i.e., GH staff).

@Olf0 Olf0 marked this pull request as draft November 11, 2024 03:14
@Olf0 Olf0 removed their request for review November 11, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants