feat(software): Reload repositories after failure #1894

lslezak · 2025-01-14T09:10:06Z

Problem

When repository refresh fails there is no way how to retry the operation
The only workaround is to select a different product and then select back the original product
This obviously won't work if the installer contains only one product

Solution

Detect that a repository failed and offer a reload action

Recording

SLES4SAP-screen0.webm

There is a new error section with "retry" link
Unfortunately the VirtualBox does not record the mouse pointer so it not obvious that the "Try again" link was clicked by mouse
For easier testing I used a local copy of the SLE repository to easily simulate unreachable repository and make it working for the retry attempt. That's why you see a different (local) repository in the recording.

Details

It turned out that it is not simple to detect a repository failure in the software page.
It reports an issue but it might contain a translated text or there might be a different issue like missing product or pattern. That means we cannot use the issues texts.
We will very likely need a repository management later anyway it makes sense to add API for fetching the current repository setup with the load status (succeeded or failed).

Tasks

DBus interface for listing the current repository setup (added the ListRepositories DBus method)
Add the HTTP API endpoint /api/software/repositories using the DBus backend above (@jreidinger thanks!)
Adapt the frontend code to show a failure
Adapt the frontend to retry after clicking a button

Use the first solution This reverts commit e425bb6.

coveralls · 2025-01-17T16:00:44Z

Pull Request Test Coverage Report for Build 12872505669

Details

1 of 21 (4.76%) changed or added relevant lines in 3 files are covered.
9 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.05%) to 70.868%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
service/lib/agama/dbus/software/manager.rb	1	4	25.0%
rust/agama-server/src/software/web.rs	0	4	0.0%
rust/agama-lib/src/software/client.rs	0	13	0.0%

Files with Coverage Reduction	New Missed Lines	%
service/lib/agama/dbus/software/manager.rb	9	80.23%

Totals
Change from base Build 12870685697:	-0.05%
Covered Lines:	17233
Relevant Lines:	24317

💛 - Coveralls

dgdavid

Web part looks good to me, just few NP comments.

dgdavid · 2025-01-18T22:53:53Z

web/src/api/storage/types/config.ts

@@ -1,4 +1,3 @@
-/* eslint-disable */


NP: I believe such a line was there because the file is automatically generated. Let's ask @imobachgs or @joseivanlopez

Yes, but I have removed it too in our feature branch. I have to check whether that line can be omitted somehow when generating the file.

web/src/components/software/SoftwarePage.tsx

joseivanlopez · 2025-01-20T09:29:45Z

service/lib/agama/dbus/software/manager.rb

@@ -67,6 +67,24 @@ def issues
        private_constant :SOFTWARE_INTERFACE

        dbus_interface SOFTWARE_INTERFACE do
+          # array of repository properties: pkg-bindings ID, alias, name, URL, product dir, enabled
+          # and loaded flag
+          dbus_method :ListRepositories, "out Result:a(issssbb)" do


In storage we are tending to other kind of interfaces. The storage API returns JSON data instead of D-Bus structs, dicts, etc. At the end, we are using D-Bus as a transport layer, and probably we will replace it in the future by a HTTP API. Having JSON data has 2 advandanges in our case:

Replacing D-Bus by HTTP will be almost transparent.

The rust code is only a proxy that defines an endpoint (it does not need to interpret the data coming from D-Bus).

I guess it is fine to continue with this approach using structs in software. Anyway, we want to rewrite this service in rust.

joseivanlopez

LGTM

imobachgs

I trust @dgdavid when it comes to the UI. The rest looks good to me.

imobachgs · 2025-01-20T10:52:23Z

rust/agama-lib/src/software/client.rs

@@ -28,6 +28,7 @@ use serde_repr::Serialize_repr;
 use std::collections::HashMap;
 use zbus::Connection;

+// TODO: move it to model?


I agree. But there is not need to do it right now.

dgdavid · 2025-01-20T12:33:38Z

web/src/components/software/SoftwarePage.tsx

  const ReloadSection = (): React.ReactNode => (
    // TRANSLATORS: title for an error message box, at least one repository could not be loaded
    <Alert variant="danger" isInline title={_("Repository load failed")}>
-      {/* TRANSLATORS: error details followed by a "Try again" link*/}
-      {_(
-        "Some installation repositories could not be loaded. The system cannot be installed without them.",
-      )}{" "}
      {loading ? (
        <>
-          {" "}
-          <Spinner size="md" />
+          <Spinner size="md" /> {_("Loading the installation repositories...")}
        </>
      ) : (
-        <Button variant="link" isInline onClick={startProbing}>
-          {/* TRANSLATORS: link for retrying failed repository load */}
-          {_("Try again")}
-        </Button>
+        <>
+          {errorMsg}{" "}
+          <Button variant="link" isInline onClick={startProbing}>
+            {/* TRANSLATORS: link for retrying failed repository load */}
+            {_("Try again")}
+          </Button>
+        </>


Sorry, I forgot to mention in yestereday's review. Please, move this component outside of the component is using it. As far as I can see yo just need to give it an onReload prop

<ReloadSection onReload={startProbing} />

Reason for moving it outside: https://react.dev/learn/preserving-and-resetting-state.

We have had issues with that, for example #1550 (comment)

Thanks a lot about this, I didn't know it!

https://react.dev/learn/preserving-and-resetting-state#different-components-at-the-same-position-reset-state

lslezak and others added 13 commits January 14, 2025 09:18

Reload repositories after failure, initial commit

98e59b1

implement rust part

479d7c8

Reload repositories

f9d3c32

Put reload into the issues

e425bb6

Revert "Put reload into the issues"

2750478

Use the first solution This reverts commit e425bb6.

Added progress indicator

d5fd4a4

Add missing Registration Error schema to openapi

f741fcc

Fixed unit test

84fc31d

Update unit test

28d19b3

Enable ESlint, it passes

fd53849

Update unit test

005f32a

Changes

7d1543f

Merge branch 'master' into repository_retry

4ca53aa

Small fixes

99d9d48

lslezak marked this pull request as ready for review January 17, 2025 16:29

dgdavid reviewed Jan 18, 2025

View reviewed changes

joseivanlopez reviewed Jan 20, 2025

View reviewed changes

joseivanlopez approved these changes Jan 20, 2025

View reviewed changes

imobachgs approved these changes Jan 20, 2025

View reviewed changes

Better reloading feedback

f2b7fcb

dgdavid reviewed Jan 20, 2025

View reviewed changes

lslezak added 2 commits January 20, 2025 15:36

Do not nest the components

c050f35

https://react.dev/learn/preserving-and-resetting-state#different-components-at-the-same-position-reset-state

Merge remote-tracking branch 'origin/master' into repository_retry

eff1a18

imobachgs approved these changes Jan 20, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into repository_retry

455ded2

lslezak merged commit 6901956 into master Jan 20, 2025
14 checks passed

lslezak deleted the repository_retry branch January 20, 2025 17:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(software): Reload repositories after failure #1894

feat(software): Reload repositories after failure #1894

lslezak commented Jan 14, 2025 •

edited

Loading

coveralls commented Jan 17, 2025 •

edited

Loading

dgdavid left a comment

dgdavid Jan 18, 2025

joseivanlopez Jan 20, 2025

joseivanlopez Jan 20, 2025

joseivanlopez left a comment

imobachgs left a comment

imobachgs Jan 20, 2025

dgdavid Jan 20, 2025

lslezak Jan 20, 2025

feat(software): Reload repositories after failure #1894

feat(software): Reload repositories after failure #1894

Conversation

lslezak commented Jan 14, 2025 • edited Loading

Problem

Solution

Recording

Details

Tasks

coveralls commented Jan 17, 2025 • edited Loading

Pull Request Test Coverage Report for Build 12872505669

Details

💛 - Coveralls

dgdavid left a comment

Choose a reason for hiding this comment

dgdavid Jan 18, 2025

Choose a reason for hiding this comment

joseivanlopez Jan 20, 2025

Choose a reason for hiding this comment

joseivanlopez Jan 20, 2025

Choose a reason for hiding this comment

joseivanlopez left a comment

Choose a reason for hiding this comment

imobachgs left a comment

Choose a reason for hiding this comment

imobachgs Jan 20, 2025

Choose a reason for hiding this comment

dgdavid Jan 20, 2025

Choose a reason for hiding this comment

lslezak Jan 20, 2025

Choose a reason for hiding this comment

lslezak commented Jan 14, 2025 •

edited

Loading

coveralls commented Jan 17, 2025 •

edited

Loading