CLI automatic answers to questions by jreidinger · Pull Request #668 · agama-project/agama

jreidinger · 2023-07-17T20:01:10Z

Problem

Add to CLI ability to use default answers for questions. As we do not have yet in place questions handling, it will be for near future only way to answer it on CLI.

Solution

Add command agama questions use-defaults. It is not as described at https://github.com/openSUSE/agama/blob/master/doc/cli_guidelines.md because currently auto answering can be only set on dbus API. Also there is risk of collision as auto answering is global for all questions, not just ones caused by given CLI command, so it can confuse user. And last but not least it can lead to some confusion where to use --non-interactive as sometimes it is hard to know what settings can cause reprobe or some action that can lead to answer.

Testing

Tested manually

Help Output

jreidinger@jreidinger:~/prace/yast/d-installer/rust> target/debug/agama questions --help
Questions handling

Usage: agama questions <COMMAND>

Commands:
  mode  Set mode for answering questions
  help  Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help
jreidinger@jreidinger:~/prace/yast/d-installer/rust> target/debug/agama questions mode --help
Set mode for answering questions

Usage: agama questions mode <VALUE>

Arguments:
  <VALUE>  [possible values: interactive, non-interactive]

Options:
  -h, --help  Print help

coveralls · 2023-07-17T20:12:59Z

coverage: 72.16% (-0.06%) from 72.219% when pulling ef7ca5e on cli_auto_questions into bd024b8 on master.

imobachgs

I am not 100% sure about the overall approach. What happens if I want to disable this behavior? Do we need a separate command? Perhaps we use a single command like agama questions mode [interactive|defaults] or something similar.

imobachgs · 2023-07-18T09:01:26Z

rust/agama-cli/src/questions.rs

+
+// TODO when more commands is added, refactor and add it to agama-lib and share a bit of functionality
+async fn use_defaults() -> anyhow::Result<()> {
+    let connection = connection().await.context("Connection to DBus failed.")?;


I think there is no need to add the context because it does not add any information to the error coming from the connection function.

jreidinger · 2023-07-18T09:09:08Z

@imobachgs issue with having interactive is that it does not need to be interactive. Basically idea is to have multiple strategies that you add to questions. So you e.g. specify few answers and then also non-interactive. This will lead to AnswerStrategy File + Default . BTW currently we do not have a way to disable using default answers. Is it needed?

imobachgs · 2023-07-18T09:14:17Z

@imobachgs issue with having interactive is that it does not need to be interactive. Basically idea is to have multiple strategies that you add to questions. So you e.g. specify few answers and then also non-interactive. This will lead to AnswerStrategy File + Default . BTW currently we do not have a way to disable using default answers. Is it needed?

Well, as said, perhaps "interactive" is not the right word. My point is that we might want to support different strategies, and I would use a single command to set them. But I do not have a strong opinion about that.

About the --non-interactive flag, we could implement that in the future by setting the strategy before running the command and rolling back to the original one after the end.

imobachgs

After discussing on IRC, we might need to think of a more flexible approach. But, by now, it is good enough. So let's unblock and continue from here.

imobachgs · 2023-07-18T09:22:03Z

rust/agama-cli/src/questions.rs

+    proxy
+        .use_default_answer()
+        .await
+        .context("Failed to set default answer")?;


Suggested change

.context("Failed to set default answer")?;

.context("Failed to set default answers strategy")?;

joseivanlopez · 2023-07-18T09:34:48Z

Sorry, I was a bit disconnected of this topic for some time, so I had to revisit it. If I got it correctly, we could have 3 different ways for answering questions: using default answers, using answers from a file, and asking to the user. And we don't know in adavance if a command could raise a question.

And then it is the problem with globally configuring the questions service, because the configuration affects to all questions, not only to the questions comming from a specific command.

Well, for the global mode issue, I think it would be difficult to avoid because a new question can be dispatched after setting the mode. I would not focus on this problem now, but maybe we could explore the possibility of temporary setting the questions mode as part of the question call, and then set it back to its previous mode.

And for settings the modes, I agree with a proposal similar to what @imobachgs exposed:

agama questions mode [interactive|non-interactive [path]]

agama questions mode interactive # default mode, ask questions
agama questions mode non-interactive # do not ask questions, use defaults
agama questions mode non-interactive /tmp/answers.json # do not ask questions, use answers from file or defaults

doc/cli_guidelines.md

imobachgs · 2023-07-18T12:26:57Z

rust/agama-cli/src/questions.rs

+    Interactive,
+    NonInteractive,
+}
+// TODO when more commands is added, refactor and add it to agama-lib and share a bit of functionality


Beware that the code which is specific to the CLI should live in agama-cli. That's the main intention of this package.

well, I mean here code that handles questions, do connection, use Question API and so on. But lets see how future will look like. For rust it is nice that strict separation of public and private, so it is easy to refactor internal stuff.

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

joseivanlopez · 2023-07-18T12:29:55Z

doc/cli_guidelines.md

 ## Non Interactive Mode

-Commands should offer a `--non-interactive` option to make scripting possible. The non interactive mode should allow answering questions automatically.
+There are questions subcommand that support `mode` subcommand to set non-interactive mode. Another option is to use `answers` subcommand to pass file with


NP: I woud rephase it as: There is a questions subcommand that support mode and answers subcommands. The mode subcommand allows to set interactive and no-interactive modes. And answers allows to pass a file with prefefined answers.

jreidinger added 3 commits July 17, 2023 15:14

update proxies with new question interface

6271551

Add command to set default answers for questions

f1bc1b0

formatting

12ee889

Add example to object path method

55815d6

imobachgs reviewed Jul 18, 2023

View reviewed changes

imobachgs approved these changes Jul 18, 2023

View reviewed changes

imobachgs changed the title ~~Cli automatic answers to questions~~ CLI automatic answers to questions Jul 18, 2023

jreidinger added 3 commits July 18, 2023 14:03

Change commands for questions

2e2c108

remove unused use

d507928

update cli guidelines

7a9c614

imobachgs approved these changes Jul 18, 2023

View reviewed changes

Apply suggestions from code review

c0ec508

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

joseivanlopez reviewed Jul 18, 2023

View reviewed changes

jreidinger added 2 commits July 18, 2023 14:52

apply suggestion from jilopez for doc

1d74a2b

fix formatting

f36e29b

jreidinger marked this pull request as ready for review July 18, 2023 13:06

joseivanlopez approved these changes Jul 18, 2023

View reviewed changes

jreidinger added 2 commits July 18, 2023 15:34

changes

b4315b1

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

ef7ca5e

jreidinger merged commit 410c673 into master Jul 18, 2023

jreidinger deleted the cli_auto_questions branch July 18, 2023 13:45

imobachgs mentioned this pull request Aug 2, 2023

Release version 3 #683

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI automatic answers to questions#668

CLI automatic answers to questions#668
jreidinger merged 12 commits intomasterfrom
cli_auto_questions

jreidinger commented Jul 17, 2023 •

edited

Loading

Uh oh!

coveralls commented Jul 17, 2023 •

edited

Loading

Uh oh!

imobachgs left a comment

Uh oh!

imobachgs Jul 18, 2023

Uh oh!

jreidinger commented Jul 18, 2023

Uh oh!

imobachgs commented Jul 18, 2023

Uh oh!

imobachgs left a comment

Uh oh!

imobachgs Jul 18, 2023

Uh oh!

joseivanlopez commented Jul 18, 2023

Uh oh!

Uh oh!

imobachgs Jul 18, 2023

Uh oh!

jreidinger Jul 18, 2023

Uh oh!

joseivanlopez Jul 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	.context("Failed to set default answer")?;
	.context("Failed to set default answers strategy")?;

Conversation

jreidinger commented Jul 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Testing

Help Output

Uh oh!

coveralls commented Jul 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imobachgs left a comment

Choose a reason for hiding this comment

Uh oh!

imobachgs Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

jreidinger commented Jul 18, 2023

Uh oh!

imobachgs commented Jul 18, 2023

Uh oh!

imobachgs left a comment

Choose a reason for hiding this comment

Uh oh!

imobachgs Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

joseivanlopez commented Jul 18, 2023

Uh oh!

Uh oh!

imobachgs Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

jreidinger Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

joseivanlopez Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jreidinger commented Jul 17, 2023 •

edited

Loading

coveralls commented Jul 17, 2023 •

edited

Loading