Skip to content

Commit

Permalink
feat(software): Reload repositories after failure (#1894)
Browse files Browse the repository at this point in the history
## 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/cd530256-c553-400a-b794-f628b06b3980)
-->

[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 <[email protected]>
  • Loading branch information
lslezak and jreidinger authored Jan 20, 2025
1 parent 31823f4 commit 6901956
Show file tree
Hide file tree
Showing 18 changed files with 250 additions and 9 deletions.
25 changes: 24 additions & 1 deletion rust/agama-lib/src/software/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -88,6 +89,28 @@ impl<'a> SoftwareClient<'a> {
})
}

/// Returns list of defined repositories
pub async fn repositories(&self) -> Result<Vec<Repository>, ServiceError> {
let repositories: Vec<Repository> = 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<Vec<Pattern>, ServiceError> {
let patterns: Vec<Pattern> = self
Expand Down
20 changes: 20 additions & 0 deletions rust/agama-lib/src/software/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
5 changes: 5 additions & 0 deletions rust/agama-lib/src/software/proxies/software.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ use zbus::proxy;
/// * Order.
pub type PatternsMap = std::collections::HashMap<String, (String, String, String, String, String)>;

pub type Repository = (i32, String, String, String, String, bool, bool);

#[proxy(
default_service = "org.opensuse.Agama.Software1",
default_path = "/org/opensuse/Agama/Software1",
Expand All @@ -77,6 +79,9 @@ pub trait Software1 {
/// ListPatterns method
fn list_patterns(&self, filtered: bool) -> zbus::Result<PatternsMap>;

/// ListRepositories method
fn list_repositories(&self) -> zbus::Result<Vec<Repository>>;

/// Probe method
fn probe(&self) -> zbus::Result<()>;

Expand Down
22 changes: 21 additions & 1 deletion rust/agama-server/src/software/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -208,6 +208,7 @@ pub async fn software_service(dbus: zbus::Connection) -> Result<Router, ServiceE
};
let router = Router::new()
.route("/patterns", get(patterns))
.route("/repositories", get(repositories))
.route("/products", get(products))
.route("/licenses", get(licenses))
.route("/licenses/:id", get(license))
Expand Down Expand Up @@ -244,6 +245,25 @@ async fn products(State(state): State<SoftwareState<'_>>) -> Result<Json<Vec<Pro
Ok(Json(products))
}

/// Returns the list of defined repositories.
///
/// * `state`: service state.
#[utoipa::path(
get,
path = "/repositories",
context_path = "/api/software",
responses(
(status = 200, description = "List of known repositories", body = Vec<Repository>),
(status = 400, description = "The D-Bus service could not perform the action")
)
)]
async fn repositories(
State(state): State<SoftwareState<'_>>,
) -> Result<Json<Vec<Repository>>, Error> {
let repositories = state.software.repositories().await?;
Ok(Json(repositories))
}

/// returns registration info
///
/// * `state`: service state.
Expand Down
6 changes: 6 additions & 0 deletions rust/agama-server/src/web/docs/software.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,15 @@ impl ApiDocBuilder for SoftwareApiDocBuilder {

fn paths(&self) -> Paths {
PathsBuilder::new()
.path_from::<crate::software::web::__path_deregister>()
.path_from::<crate::software::web::__path_get_config>()
.path_from::<crate::software::web::__path_get_registration>()
.path_from::<crate::software::web::__path_patterns>()
.path_from::<crate::software::web::__path_probe>()
.path_from::<crate::software::web::__path_products>()
.path_from::<crate::software::web::__path_proposal>()
.path_from::<crate::software::web::__path_repositories>()
.path_from::<crate::software::web::__path_register>()
.path_from::<crate::software::web::__path_set_config>()
.path_from::<crate::software::web::__path_set_resolvables>()
.build()
Expand All @@ -51,10 +55,12 @@ impl ApiDocBuilder for SoftwareApiDocBuilder {
.schema_from::<agama_lib::software::Pattern>()
.schema_from::<agama_lib::software::model::RegistrationInfo>()
.schema_from::<agama_lib::software::model::RegistrationParams>()
.schema_from::<agama_lib::software::model::RegistrationError>()
.schema_from::<agama_lib::software::model::ResolvableParams>()
.schema_from::<agama_lib::software::model::ResolvableType>()
.schema_from::<agama_lib::software::SelectedBy>()
.schema_from::<agama_lib::software::model::SoftwareConfig>()
.schema_from::<agama_lib::software::model::Repository>()
.schema_from::<crate::software::web::SoftwareProposal>()
.schema_from::<crate::web::common::Issue>()
.build()
Expand Down
7 changes: 7 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Mon Jan 20 16:44:02 UTC 2025 - Ladislav Slezák <[email protected]>

- 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 <[email protected]>

Expand Down
18 changes: 18 additions & 0 deletions service/lib/agama/dbus/software/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
[
Expand Down
3 changes: 2 additions & 1 deletion service/lib/agama/software/repositories_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions service/lib/agama/software/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Mon Jan 20 16:44:45 UTC 2025 - Ladislav Slezák <[email protected]>

- 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 <[email protected]>

Expand Down
4 changes: 2 additions & 2 deletions service/test/agama/software/repositories_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
7 changes: 7 additions & 0 deletions web/package/agama-web-ui.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Mon Jan 20 16:45:18 UTC 2025 - Ladislav Slezák <[email protected]>

- 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 <[email protected]>

Expand Down
13 changes: 13 additions & 0 deletions web/src/api/software.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
Product,
SoftwareConfig,
RegistrationInfo,
Repository,
SoftwareProposal,
License,
LicenseContent,
Expand Down Expand Up @@ -67,13 +68,23 @@ const fetchRegistration = (): Promise<RegistrationInfo> => get("/api/software/re
*/
const fetchPatterns = (): Promise<Pattern[]> => get("/api/software/patterns");

/**
* Returns the list of configured repositories
*/
const fetchRepositories = (): Promise<Repository[]> => get("/api/software/repositories");

/**
* Updates the software configuration
*
* @param config - New software configuration
*/
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
*/
Expand All @@ -88,6 +99,8 @@ export {
fetchLicenses,
fetchLicense,
fetchRegistration,
fetchRepositories,
updateConfig,
probe,
register,
};
1 change: 0 additions & 1 deletion web/src/api/storage/types/config.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
2 changes: 2 additions & 0 deletions web/src/components/software/SoftwarePage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ jest.mock("~/queries/software", () => ({
usePatterns: () => testingPatterns,
useProposal: () => testingProposal,
useProposalChanges: jest.fn(),
useRepositories: () => [],
useRepositoryMutation: () => ({ mutate: jest.fn() }),
}));

describe("SoftwarePage", () => {
Expand Down
Loading

0 comments on commit 6901956

Please sign in to comment.