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

feat: unify progress handling #1356

Merged
merged 10 commits into from
Jun 20, 2024
Merged

feat: unify progress handling #1356

merged 10 commits into from
Jun 20, 2024

Conversation

imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Jun 19, 2024

Trello: https://trello.com/c/Miuvai6N/3706-5-better-progress-report-during-installation

The main goal of this PR is to implement a better mechanism for displaying progress in the web UI. It takes the product selection progress as inspiration but eliminates hardcoded values.

Progress API problem

When we designed the initial progress API, Agama was quite different. While working on this PR, we have identified a relevant problem: there is no way to have a subprogress in the same service.

The manager service defines the main steps (e.g., "Probing Software") and the details are just the progress from software and storage services (e.g., "Refreshing repositories metadata").

As a consequence, we cannot have a detailed "Configuring system" step at the end of the installation with its details (e.g., "Writing users"). You can only have either 1) a "Configuring system" with no details or several separate steps, which is too much.

In general, we are telling people that they can use the WS to track the progress, so we should decide on a stable API as soon as possible.

Screenshots

Installation progress

Captura desde 2024-06-19 15-07-24

Product selection selection progress

As you may see in the following screenshot, some texts have been changed. The reason is that now it uses the texts from the backend. We should consider adjusting them.
Captura desde 2024-06-19 15-06-57

@imobachgs imobachgs force-pushed the better-progress branch 2 times, most recently from 95f1adb to 666fa0e Compare June 19, 2024 12:04
@coveralls
Copy link

Coverage Status

coverage: 70.872% (-0.007%) from 70.879%
when pulling 666fa0e on better-progress
into c516de3 on master.

@imobachgs imobachgs marked this pull request as ready for review June 19, 2024 14:10
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@agama-project agama-project deleted a comment from coveralls Jun 19, 2024
@@ -132,11 +132,11 @@ def probe
logger.info "Probing software"

if repositories.empty?
start_progress(3)
start_progress_with_size(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case the descriptions are known in advance. Should we use the with_descritptions constructor?

}, [cancellablePromise, setSoftwareProgress, software]);
if (!selectedProduct) {
return;
}

if (status === IDLE) return <Navigate to="/" replace />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably we could move this navigation to App:


    if (status === IDLE && location.pathname === "/products/progress") {
      return <Navigate to="/" />;
    }

So App would contain all the "routing" logic until we will improve the router usage.

Copy link
Contributor Author

@imobachgs imobachgs Jun 20, 2024

Choose a reason for hiding this comment

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

I see your point, but it is not part of this PR and, most importantly, I am scared of breaking the navigation again 😅. I would keep it as it is until we fix the router usage.

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@imobachgs imobachgs merged commit 7b4db54 into master Jun 20, 2024
6 checks passed
@imobachgs imobachgs deleted the better-progress branch June 20, 2024 12:02
@imobachgs imobachgs added this to the Agama 9 milestone Jun 25, 2024
@imobachgs imobachgs mentioned this pull request Jun 27, 2024
imobachgs added a commit that referenced this pull request Jun 27, 2024
Prepare for releasing Agama 9. It includes the following pull requests:

- #1101
- #1202
- #1228
- #1231
- #1236
- #1238
- #1239
- #1240
- #1242
- #1243
- #1244
- #1245
- #1246
- #1247
- #1248
- #1249
- #1250
- #1251
- #1252
- #1253
- #1254
- #1255
- #1256
- #1257
- #1258
- #1259
- #1260
- #1261
- #1264
- #1265
- #1267
- #1268
- #1269
- #1270
- #1271
- #1272
- #1273
- #1274
- #1279
- #1280
- #1284
- #1285
- #1286
- #1287
- #1288
- #1289
- #1290
- #1291
- #1292
- #1293
- #1294
- #1295
- #1296
- #1298
- #1299
- #1300
- #1301
- #1302
- #1303
- #1304
- #1305
- #1306
- #1307
- #1308
- #1309
- #1310
- #1311
- #1312
- #1313
- #1314
- #1315
- #1316
- #1317
- #1318
- #1319
- #1320
- #1321
- #1322
- #1323
- #1324
- #1325
- #1326
- #1328
- #1329
- #1331
- #1332
- #1334
- #1338
- #1340
- #1341
- #1342
- #1343
- #1344
- #1345
- #1348
- #1349
- #1351
- #1352
- #1353
- #1354
- #1355
- #1356
- #1357
- #1358
- #1359
- #1360
- #1361
- #1362
- #1363
- #1365
- #1366
- #1367
- #1368
- #1371
- #1372
- #1374
- #1375
- #1376
- #1379
- #1380
- #1381
- #1383
- #1384
- #1385
- #1386
- #1387
- #1388
- #1389
- #1391
- #1392
- #1394
- #1395
- #1397
- #1398
- #1399
- #1400
- #1403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants