From 69019564c9a63c4f5f7b8188fc023f939229ddef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Mon, 20 Jan 2025 18:01:43 +0100 Subject: [PATCH] feat(software): Reload repositories after failure (#1894) ## Problem ![agama-repo-failed](https://github.com/user-attachments/assets/522bb11c-9698-4278-816e-b472f25d73f1) - 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](https://github.com/user-attachments/assets/ad9a5798-5f42-4333-8a4a-9419953a85ee) - 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 - [x] DBus interface for listing the current repository setup (added the `ListRepositories` DBus method) - [x] Add the HTTP API endpoint `/api/software/repositories` using the DBus backend above (@jreidinger thanks!) - [x] Adapt the frontend code to show a failure - [x] Adapt the frontend to retry after clicking a button --------- Co-authored-by: Josef Reidinger --- rust/agama-lib/src/software/client.rs | 25 +++++++- rust/agama-lib/src/software/model.rs | 20 ++++++ .../src/software/proxies/software.rs | 5 ++ rust/agama-server/src/software/web.rs | 22 ++++++- rust/agama-server/src/web/docs/software.rs | 6 ++ rust/package/agama.changes | 7 ++ service/lib/agama/dbus/software/manager.rb | 18 ++++++ .../agama/software/repositories_manager.rb | 3 +- service/lib/agama/software/repository.rb | 8 +++ service/package/rubygem-agama-yast.changes | 6 ++ .../software/repositories_manager_test.rb | 4 +- web/package/agama-web-ui.changes | 7 ++ web/src/api/software.ts | 13 ++++ web/src/api/storage/types/config.ts | 1 - .../components/software/SoftwarePage.test.tsx | 2 + web/src/components/software/SoftwarePage.tsx | 64 ++++++++++++++++++- web/src/queries/software.ts | 37 +++++++++++ web/src/types/software.ts | 11 ++++ 18 files changed, 250 insertions(+), 9 deletions(-) diff --git a/rust/agama-lib/src/software/client.rs b/rust/agama-lib/src/software/client.rs index 26b63e48e3..f868235a93 100644 --- a/rust/agama-lib/src/software/client.rs +++ b/rust/agama-lib/src/software/client.rs @@ -19,7 +19,7 @@ // find current contact information at www.suse.com. use super::{ - model::ResolvableType, + model::{Repository, ResolvableType}, proxies::{ProposalProxy, Software1Proxy}, }; use crate::error::ServiceError; @@ -28,6 +28,7 @@ use serde_repr::Serialize_repr; use std::collections::HashMap; use zbus::Connection; +// TODO: move it to model? /// Represents a software product #[derive(Debug, Serialize, utoipa::ToSchema)] pub struct Pattern { @@ -88,6 +89,28 @@ impl<'a> SoftwareClient<'a> { }) } + /// Returns list of defined repositories + pub async fn repositories(&self) -> Result, ServiceError> { + let repositories: Vec = self + .software_proxy + .list_repositories() + .await? + .into_iter() + .map( + |(id, alias, name, url, product_dir, enabled, loaded)| Repository { + id, + alias, + name, + url, + product_dir, + enabled, + loaded, + }, + ) + .collect(); + Ok(repositories) + } + /// Returns the available patterns pub async fn patterns(&self, filtered: bool) -> Result, ServiceError> { let patterns: Vec = self diff --git a/rust/agama-lib/src/software/model.rs b/rust/agama-lib/src/software/model.rs index 8556dae11d..bb290803fd 100644 --- a/rust/agama-lib/src/software/model.rs +++ b/rust/agama-lib/src/software/model.rs @@ -98,3 +98,23 @@ pub struct ResolvableParams { /// Whether the resolvables are optional or not. pub optional: bool, } + +/// Repository list specification. +#[derive(Deserialize, Serialize, utoipa::ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct Repository { + /// repository identifier + pub id: i32, + /// repository alias. Has to be unique + pub alias: String, + /// repository name + pub name: String, + /// Repository url (raw format without expanded variables) + pub url: String, + /// product directory (currently not used, valid only for multiproduct DVDs) + pub product_dir: String, + /// Whether the repository is enabled + pub enabled: bool, + /// Whether the repository is loaded + pub loaded: bool, +} diff --git a/rust/agama-lib/src/software/proxies/software.rs b/rust/agama-lib/src/software/proxies/software.rs index 51b067e91d..02dc1040c7 100644 --- a/rust/agama-lib/src/software/proxies/software.rs +++ b/rust/agama-lib/src/software/proxies/software.rs @@ -52,6 +52,8 @@ use zbus::proxy; /// * Order. pub type PatternsMap = std::collections::HashMap; +pub type Repository = (i32, String, String, String, String, bool, bool); + #[proxy( default_service = "org.opensuse.Agama.Software1", default_path = "/org/opensuse/Agama/Software1", @@ -77,6 +79,9 @@ pub trait Software1 { /// ListPatterns method fn list_patterns(&self, filtered: bool) -> zbus::Result; + /// ListRepositories method + fn list_repositories(&self) -> zbus::Result>; + /// Probe method fn probe(&self) -> zbus::Result<()>; diff --git a/rust/agama-server/src/software/web.rs b/rust/agama-server/src/software/web.rs index 7831def87b..99c7e95d22 100644 --- a/rust/agama-server/src/software/web.rs +++ b/rust/agama-server/src/software/web.rs @@ -39,7 +39,7 @@ use agama_lib::{ product::{proxies::RegistrationProxy, Product, ProductClient}, software::{ model::{ - RegistrationError, RegistrationInfo, RegistrationParams, ResolvableParams, + RegistrationError, RegistrationInfo, RegistrationParams, Repository, ResolvableParams, SoftwareConfig, }, proxies::{Software1Proxy, SoftwareProductProxy}, @@ -208,6 +208,7 @@ pub async fn software_service(dbus: zbus::Connection) -> Result>) -> Result), + (status = 400, description = "The D-Bus service could not perform the action") + ) +)] +async fn repositories( + State(state): State>, +) -> Result>, Error> { + let repositories = state.software.repositories().await?; + Ok(Json(repositories)) +} + /// returns registration info /// /// * `state`: service state. diff --git a/rust/agama-server/src/web/docs/software.rs b/rust/agama-server/src/web/docs/software.rs index 457cac6c0c..e6155cffd0 100644 --- a/rust/agama-server/src/web/docs/software.rs +++ b/rust/agama-server/src/web/docs/software.rs @@ -34,11 +34,15 @@ impl ApiDocBuilder for SoftwareApiDocBuilder { fn paths(&self) -> Paths { PathsBuilder::new() + .path_from::() .path_from::() + .path_from::() .path_from::() .path_from::() .path_from::() .path_from::() + .path_from::() + .path_from::() .path_from::() .path_from::() .build() @@ -51,10 +55,12 @@ impl ApiDocBuilder for SoftwareApiDocBuilder { .schema_from::() .schema_from::() .schema_from::() + .schema_from::() .schema_from::() .schema_from::() .schema_from::() .schema_from::() + .schema_from::() .schema_from::() .schema_from::() .build() diff --git a/rust/package/agama.changes b/rust/package/agama.changes index b5e50d6a8a..6c166b0533 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Jan 20 16:44:02 UTC 2025 - Ladislav Slezák + +- The web server provides /api/software/repositories endpoint + for reading the currently configured repositories, + related to (gh#agama-project/agama#1894) + ------------------------------------------------------------------- Mon Jan 20 10:35:47 UTC 2025 - Imobach Gonzalez Sosa diff --git a/service/lib/agama/dbus/software/manager.rb b/service/lib/agama/dbus/software/manager.rb index 1ebd0b0088..b8fd7d8f5c 100644 --- a/service/lib/agama/dbus/software/manager.rb +++ b/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 + backend.repositories.repositories.map do |repo| + [ + [ + repo.repo_id, + repo.repo_alias, + repo.name, + repo.raw_url.uri.to_s, + repo.product_dir, + repo.enabled?, + !!repo.loaded? + ] + ] + end + end + # value of result hash is category, description, icon, summary and order dbus_method :ListPatterns, "in Filtered:b, out Result:a{s(sssss)}" do |filtered| [ diff --git a/service/lib/agama/software/repositories_manager.rb b/service/lib/agama/software/repositories_manager.rb index b963260163..63a4d08eef 100644 --- a/service/lib/agama/software/repositories_manager.rb +++ b/service/lib/agama/software/repositories_manager.rb @@ -65,12 +65,13 @@ def disabled # Loads the repository metadata # # As a side effect, it disables those repositories that cannot be read. - # The intentation is to prevent the proposal from trying to read them + # The intent is to prevent the proposal from trying to read them # again. def load repositories.each do |repo| if repo.probe repo.enable! + repo.refresh else repo.disable! end diff --git a/service/lib/agama/software/repository.rb b/service/lib/agama/software/repository.rb index 163c1474f9..e3ba791cfe 100644 --- a/service/lib/agama/software/repository.rb +++ b/service/lib/agama/software/repository.rb @@ -38,6 +38,14 @@ def probe type = Yast::Pkg.RepositoryProbe(url.to_s, product_dir) !!type && type != "NONE" end + + def loaded? + @loaded + end + + def refresh + @loaded = !!super + end end end end diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index a7e5490a01..d3e017c542 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Jan 20 16:44:45 UTC 2025 - Ladislav Slezák + +- The software service provides DBus API for reading the currently + configured repositories, related to (gh#agama-project/agama#1894) + ------------------------------------------------------------------- Mon Jan 20 10:35:35 UTC 2025 - Imobach Gonzalez Sosa diff --git a/service/test/agama/software/repositories_manager_test.rb b/service/test/agama/software/repositories_manager_test.rb index 9b2c106a8c..4a5983d369 100644 --- a/service/test/agama/software/repositories_manager_test.rb +++ b/service/test/agama/software/repositories_manager_test.rb @@ -25,7 +25,7 @@ describe Agama::Software::RepositoriesManager do let(:repo) do instance_double( - Agama::Software::Repository, enable!: nil, probe: true, enabled?: true + Agama::Software::Repository, enable!: nil, probe: true, enabled?: true, refresh: false ) end @@ -47,7 +47,7 @@ describe "#load" do let(:repo1) do instance_double( - Agama::Software::Repository, disable!: nil, probe: false + Agama::Software::Repository, disable!: nil, probe: false, refresh: false ) end diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index db12793ba8..63ec4ae1c9 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Jan 20 16:45:18 UTC 2025 - Ladislav Slezák + +- The software page displays a link for reloading the repositories + again if some repository failed to load + (gh#agama-project/agama#1894) + ------------------------------------------------------------------- Mon Jan 20 10:36:09 UTC 2025 - Imobach Gonzalez Sosa diff --git a/web/src/api/software.ts b/web/src/api/software.ts index 568a0db011..b190c398d2 100644 --- a/web/src/api/software.ts +++ b/web/src/api/software.ts @@ -25,6 +25,7 @@ import { Product, SoftwareConfig, RegistrationInfo, + Repository, SoftwareProposal, License, LicenseContent, @@ -67,6 +68,11 @@ const fetchRegistration = (): Promise => get("/api/software/re */ const fetchPatterns = (): Promise => get("/api/software/patterns"); +/** + * Returns the list of configured repositories + */ +const fetchRepositories = (): Promise => get("/api/software/repositories"); + /** * Updates the software configuration * @@ -74,6 +80,11 @@ const fetchPatterns = (): Promise => get("/api/software/patterns"); */ const updateConfig = (config: SoftwareConfig) => put("/api/software/config", config); +/** + * Updates the software configuration + */ +const probe = () => post("/api/software/probe"); + /** * Request registration of selected product with given key */ @@ -88,6 +99,8 @@ export { fetchLicenses, fetchLicense, fetchRegistration, + fetchRepositories, updateConfig, + probe, register, }; diff --git a/web/src/api/storage/types/config.ts b/web/src/api/storage/types/config.ts index 7a7864a01d..dbaaa13af1 100644 --- a/web/src/api/storage/types/config.ts +++ b/web/src/api/storage/types/config.ts @@ -1,4 +1,3 @@ -/* eslint-disable */ /** * This file was automatically generated by json-schema-to-typescript. * DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file, diff --git a/web/src/components/software/SoftwarePage.test.tsx b/web/src/components/software/SoftwarePage.test.tsx index 132d5b0e23..60a9845c4d 100644 --- a/web/src/components/software/SoftwarePage.test.tsx +++ b/web/src/components/software/SoftwarePage.test.tsx @@ -40,6 +40,8 @@ jest.mock("~/queries/software", () => ({ usePatterns: () => testingPatterns, useProposal: () => testingProposal, useProposalChanges: jest.fn(), + useRepositories: () => [], + useRepositoryMutation: () => ({ mutate: jest.fn() }), })); describe("SoftwarePage", () => { diff --git a/web/src/components/software/SoftwarePage.tsx b/web/src/components/software/SoftwarePage.tsx index f22a2ae263..4fcf1e360f 100644 --- a/web/src/components/software/SoftwarePage.tsx +++ b/web/src/components/software/SoftwarePage.tsx @@ -20,20 +20,29 @@ * find current contact information at www.suse.com. */ -import React from "react"; +import React, { useState } from "react"; import { + Alert, + Button, DescriptionList, DescriptionListDescription, DescriptionListGroup, DescriptionListTerm, Grid, GridItem, + Spinner, Stack, } from "@patternfly/react-core"; import { Link, IssuesHint, Page } from "~/components/core"; import UsedSize from "./UsedSize"; import { useIssues } from "~/queries/issues"; -import { usePatterns, useProposal, useProposalChanges } from "~/queries/software"; +import { + usePatterns, + useProposal, + useProposalChanges, + useRepositories, + useRepositoryMutation, +} from "~/queries/software"; import { Pattern, SelectedBy } from "~/types/software"; import { _ } from "~/i18n"; import { SOFTWARE as PATHS } from "~/routes/paths"; @@ -86,6 +95,39 @@ const NoPatterns = (): React.ReactNode => ( ); +const errorMsg = _( + /* TRANSLATORS: error details followed by a "Try again" link*/ + "Some installation repositories could not be loaded. \ +The system cannot be installed without them.", +); + +// error message, allow reloading the repositories again +const ReloadSection = ({ + loading, + action, +}: { + loading: boolean; + action: () => void; +}): React.ReactNode => ( + // TRANSLATORS: title for an error message box, at least one repository could not be loaded + + {loading ? ( + <> + {/* TRANSLATORS: progress message */} + {_("Loading the installation repositories...")} + + ) : ( + <> + {errorMsg}{" "} + + + )} + +); + /** * Software page component */ @@ -93,13 +135,24 @@ function SoftwarePage(): React.ReactNode { const issues = useIssues("software"); const proposal = useProposal(); const patterns = usePatterns(); + const repos = useRepositories(); + + const [loading, setLoading] = useState(false); + const { mutate: probe } = useRepositoryMutation(() => setLoading(false)); useProposalChanges(); // Selected patterns section should fill the full width in big screen too when - // tehere is no information for rendering the Proposal Size section. + // there is no information for rendering the Proposal Size section. const selectedPatternsXlSize = proposal.size ? 6 : 12; + const startProbing = () => { + setLoading(true); + probe(); + }; + + const showReposAlert = repos.some((r) => !r.loaded); + return ( @@ -111,6 +164,11 @@ function SoftwarePage(): React.ReactNode { + {showReposAlert && ( + + + + )} {patterns.length === 0 ? : } diff --git a/web/src/queries/software.ts b/web/src/queries/software.ts index 2aea9510e0..3117bca5f4 100644 --- a/web/src/queries/software.ts +++ b/web/src/queries/software.ts @@ -36,6 +36,7 @@ import { PatternsSelection, Product, RegistrationInfo, + Repository, SelectedBy, SoftwareConfig, SoftwareProposal, @@ -47,6 +48,8 @@ import { fetchProducts, fetchProposal, fetchRegistration, + fetchRepositories, + probe, register, updateConfig, } from "~/api/software"; @@ -111,6 +114,14 @@ const patternsQuery = () => ({ queryFn: fetchPatterns, }); +/** + * Query to retrieve configured repositories + */ +const repositoriesQuery = () => ({ + queryKey: ["software/repositories"], + queryFn: fetchRepositories, +}); + /** * Hook that builds a mutation to update the software configuration * @@ -153,6 +164,22 @@ const useRegisterMutation = () => { return useMutation(query); }; +/** + * Hook that builds a mutation for reloading repositories + */ +const useRepositoryMutation = (callback: () => void) => { + const queryClient = useQueryClient(); + + const query = { + mutationFn: probe, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["software/repositories"] }); + callback(); + }, + }; + return useMutation(query); +}; + /** * Returns available products and selected one, if any */ @@ -230,6 +257,14 @@ const useRegistration = (): RegistrationInfo => { return registration; }; +/** + * Returns repository info + */ +const useRepositories = (): Repository[] => { + const { data: repositories } = useSuspenseQuery(repositoriesQuery()); + return repositories; +}; + /** * Hook that returns a useEffect to listen for software proposal events * @@ -287,4 +322,6 @@ export { useProposalChanges, useRegistration, useRegisterMutation, + useRepositories, + useRepositoryMutation, }; diff --git a/web/src/types/software.ts b/web/src/types/software.ts index 18972a39d1..ea9337a537 100644 --- a/web/src/types/software.ts +++ b/web/src/types/software.ts @@ -94,6 +94,16 @@ type Pattern = { selectedBy?: SelectedBy; }; +type Repository = { + repo_id: number; + alias: string; + name: string; + raw_url: string; + product_dir: string; + enabled: boolean; + loaded: boolean; +}; + type RegistrationInfo = { key: string; email?: string; @@ -108,5 +118,6 @@ export type { LicenseContent, SoftwareConfig, RegistrationInfo, + Repository, SoftwareProposal, };