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(cli): agama profile autoyast command #1279

Merged
merged 9 commits into from
Jun 3, 2024
Merged

Conversation

jreidinger
Copy link
Contributor

Problem

Call agama profile download do for autoyast beside downloading also convert to json which is confusing to use e.g. agama profile download ftp://test.com/autoyast.xml > profile.json

Solution

For downloading use agama download and for special autoyast processing regarding URL and also result use agama profile autoyast

@coveralls
Copy link

coveralls commented May 31, 2024

Coverage Status

coverage: 70.281% (-0.01%) from 70.293%
when pulling 8881612 on cli_autoyast_convert
into 903101b on master.

@imobachgs imobachgs changed the title Feat(CLI): agama profile autoyast command feat(cli): agama profile autoyast command Jun 3, 2024
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

It looks good, but the documentation could be improved (added a few suggestions). Additionally, I guess we should update the changes file.

rust/agama-cli/src/commands.rs Outdated Show resolved Hide resolved
rust/agama-cli/src/commands.rs Outdated Show resolved Hide resolved
rust/agama-cli/src/commands.rs Outdated Show resolved Hide resolved
rust/agama-cli/src/profile.rs Outdated Show resolved Hide resolved
rust/agama-cli/src/profile.rs Outdated Show resolved Hide resolved
rust/agama-cli/src/profile.rs Outdated Show resolved Hide resolved
rust/agama-cli/src/profile.rs Outdated Show resolved Hide resolved
let mut transfer = handle.transfer();
transfer.write_function(|buf|
// unwrap here is ok, as we want to kill download if we failed to write content
Ok(out_fd.write(buf).unwrap()))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we get a proper error? Not tried, just asking...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in debug build for sure...regarding release I have to try it

}

pub fn read(&self) -> anyhow::Result<String> {
pub fn read(&self, mut out_fd: impl Write) -> anyhow::Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

np: I understand the reason, but looks weird that the function is called "read" and receives a "impl Write" argument :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can name it read_into?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it.

rust/agama-lib/src/profile.rs Outdated Show resolved Hide resolved
@jreidinger jreidinger merged commit e2c7a5a into master Jun 3, 2024
2 checks passed
@jreidinger jreidinger deleted the cli_autoyast_convert branch June 3, 2024 08:02
joseivanlopez added a commit that referenced this pull request Jun 5, 2024
The list of drives in the "partitioning" section of an AutoYaST profile
is directly converted to json as "legacyAutoyastStorage" key.

See:

* #1279
* #1284

~~~
$ ./service/bin/agama-autoyast file:///path/to/profile.xml /tmp
$ cat /tmp/autoinst.json | jq

{
  "legacyAutoyastStorage": [
    {
      "device": "/dev/sda",
      "disklabel": "gpt",
      "enable_snapshots": true,
      "initialize": true,
      "partitions": [
        {
          "create": true,
          "partition_id": 263,
          "partition_nr": 1,
          "resize": false,
          "size": "8225280"
        }
      ],
      "type": "CT_DISK",
      "use": "all"
  ]
}
~~~

Note: Agama CLI complains when using `file://`:

~~~
$ agama profile autoyast file:///path/to/profile.xml
No such file or directory (os error 2)
~~~
@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