Skip to content

Conversation

@adrianreber
Copy link

@adrianreber adrianreber commented Aug 1, 2025

Add --no-confirm persistent flag to config command that automatically creates config files and parent directories without prompting for confirmation.

Implementation details:

  • Add --no-confirm flag to config command as persistent flag
  • Update askToCreate function to accept noConfirm parameter
  • When noConfirm is true, askToCreate returns true without prompting
  • Simplify handleFileCreation to pass flag directly to askToCreate
  • Centralize all noConfirm logic within askToCreate function
  • Maintain backward compatibility with existing askToCreate behavior
  • Add test case for noConfirm functionality

🤖 Generated with Claude Code

This PR was required by lines from the OpenHPC OpenCHAMI recipe:

$ yes | ochami config cluster set --system --default ${compute_prefix} cluster.uri https://${dns_name}:8443

being able to run

$ ochami config --no-confirm cluster set --system --default ${compute_prefix} cluster.uri https://${dns_name}:8443

looks nicer.

@davidallendj
Copy link
Contributor

davidallendj commented Aug 1, 2025

Seems like a nice little feature. What happens if you run the same command with a config that already exists? Do it overwrite, prompt to overwrite, or fail?

@adrianreber
Copy link
Author

adrianreber commented Aug 1, 2025

In my case the command finishes silently if the file already exists:
Before:

$ ./ochami config cluster set --system --default c cluster.uri https://name:8443
/etc/ochami/config.yaml does not exist. Create it? [yn]:y
$ ./ochami config cluster set --system --default c cluster.uri https://name:8444

No question on the second run. With --create:

$ rm -rf /etc/ochami/
$ ./ochami config --create cluster set --system --default c cluster.uri https://name:8443
$ ls /etc/ochami/
config.yaml
$ ./ochami config --create cluster set --system --default c cluster.uri https://name:8444

No complaints on the second run if --create is set.

Do it overwrite, prompt to overwrite, or fail?

It behaves as before. Completely recreating the file could also be expected, that is true.

Initially I wanted to call the parameter -y|--yes, which would just answer questions with yes. Not sure what is more user-friendly.

@davidallendj
Copy link
Contributor

I would think if a system config file already exists, I might want to go ahead and use it instead of overwriting it, especially since it may have been created previously by someone or something else. On the other hand, creating a prompt when you want to run everything automatically, seems counterintuitive in this particular context.

Maybe it would make sense to include another flag if the intention is to overwrite regardless of the file exists in case you use the new file and have it fail or ignore by default? I'm not exactly sure what the best default behavior should be, but I can imagine a scenario where I'd only want the new file if it doesn't exist. Just something to think about.

I also think having a -y|--yes flag for prompts would make sense IMO since it should never be destructive in this case.

@synackd
Copy link
Collaborator

synackd commented Aug 1, 2025

@adrianreber Thank you very much for your contribution! I agree that having a way to non-interactively create the config file(s) will be useful for scripting environments as you've stated. However, we should think carefully about how we do this.

Currently, there is already a --no-confirm flag for commands that delete things which deletes without prompting. IMO, we could re-use this flag for config commands. The only question would be, as stated already, should this always overwrite existing config file(s) or only if one doesn't exist? We could potentially solve this with a corresponding --confirm flag. That way, --no-confirm would automatically not create if existing (and throw an error if trying to write to a file that doesn't exist) while --confirm would.

On this, my thoughts are these:

  • When executing commands that create/modify (config set , config cluster set), the user likely wants the operation to be idempotent, creating the file if non-existent or modifying any existing file.
    • Both --confirm and --no-confirm might make sense here. If the file doesn't exist with --no-confirm, an error would be produced.
  • When executing commands that show (config [cluster] show), the user likely wants to see the merged config (default) unless --config, --system, or --user are passed. It doesn't really make sense to create a config file if any of those flags are passed.
    • The only exception I could foresee is if the user wanted to bootstrap a config file at any of those paths using the default config, but I think it would be better to have a separate command (e.g. bootstrap) to do this or just have the user do ochami config show | tee <file>.
    • Both --confirm and --no-confirm probably do not make sense here.
  • When executing commands that delete (config [cluster] unset), the user likely expects the file to already exist and so it wouldn't make sense to create a file in this case.
    • Unless the user wants to create the file only to unset something, I do not think --confirm and --no-confirm make sense here.

Long story short, it probably only makes sense to have confirm flags on config [cluster] set commands.

Add --no-confirm persistent flag to config command that automatically creates
config files and parent directories without prompting for confirmation.

Implementation details:
- Add --no-confirm flag to config command as persistent flag
- Update askToCreate function to accept noConfirm parameter
- When noConfirm is true, askToCreate returns true without prompting
- Simplify handleFileCreation to pass flag directly to askToCreate
- Centralize all noConfirm logic within askToCreate function
- Maintain backward compatibility with existing askToCreate behavior
- Add test case for noConfirm functionality

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
@adrianreber adrianreber changed the title feat(config): add --create flag to automatically create config files feat(config): add --no-confirm flag to automatically create config files Aug 3, 2025
@adrianreber
Copy link
Author

Thanks for the very detailed feedback. I tried to update the PR to have a flag called --no-confirm. I hope I understood your suggestions correctly. Let me know if this makes more sense now.

@synackd
Copy link
Collaborator

synackd commented Aug 6, 2025

Apologies for the delay. It looks like you've implemented --no-confirm to non-interactively ignore file creation, but I would imagine some users would want the option to create it non-interactively if it doesn't already exist. Could you add --confirm as well? This will require reworking the askToCreate() function header, adding more unit tests for the extra cases, and the logic within handleFileCreation().

@adrianreber
Copy link
Author

Apologies for the delay. It looks like you've implemented --no-confirm to non-interactively ignore file creation, but I would imagine some users would want the option to create it non-interactively if it doesn't already exist. Could you add --confirm as well? This will require reworking the askToCreate() function header, adding more unit tests for the extra cases, and the logic within handleFileCreation().

I am sorry, I don't get it. The flag --no-confirm skips asking the [y/n] question and always creates the file, if it doesn't exist. From my understanding, --confirm would be the way it currently works. It asks for confirmation. I have the feeling that we might have a different understanding of the --confirm, --no-confirm flag.

For me --no-confirm means that the tool should not ask for confirmation. Does --no-confirm mean for you it should always say no?

For me --confirm could mean to always ask for confirmation to change anything. Which would be a different behaviour than currently. The flag could, however, also mean that all questions should be automatically confirmed.

To me it seems the combination of --confirm and --no-confirm can mean different things. Maybe the flag needs a different name.

Before doing any further changes I hope to get some clarification from your side.

@synackd
Copy link
Collaborator

synackd commented Aug 7, 2025

Apologies, the following is sort of me thinking in writing, but hopefully it helps direct the discussion. 🙂

You bring up a good point and as I reconsider how to handle this, I realize that the current decision path for the prompting of the config file creation actually differs from that of the deletion commands.

When --no-confirm was written for the deletion commands, it was intended for the purpose you describe: "don't prompt, simply delete" (so conversely, a --confirm prompt, if implemented, would force the prompt). So the decision paths for deletion are currently:

  • Prompt user for deletion (no flag or hypothetical --confirm flag)
  • Do not prompt and delete (--no-confirm)

However, for config file creation, I think that the expected decision paths should be:

  • Do not prompt and create if non-existent
  • Do not prompt and err if non-existent (is this realistic?)
  • Prompt if non-existent

Ideally, I would like to re-use flags as much as possible to avoid having different flags that do identical/similar things. That said, I think we can rework both the config flag prompting mechanism and deletion prompting mechanism via the following generalized paths:

  • Prompt (default behavior; no flag passed or -p|--prompt)
  • Non-interactively perform "yes" option (-Y|--yes)
  • Non-interactively perform "no" option (-N|--no [-n is taken for --nid (should we change?), so using upper case here])

Applying this to the deletion example:

  • Prompt for deletion (default behavior or --prompt passed)
  • Non-interactively ("force") delete (--yes passed)
  • Non-interactively do not delete (--no passed, though this is probably useless so probably doesn't need to be implemented)

Applying this to the config creation example:

  • Prompt to create if non-existent (default behavior or --prompt passed)
  • Non-interactively create if non-existent (--yes passed)
  • Non-interactively do not create and err if non-existent (--no passed)

This allows us to reuse the same flags for commands that require yes/no prompts, subtracting as necessary, and (hopefully) reduces confusion.

What are folks' thoughts on this? If this is feasible, we can change the config options in this PR and open a subsequent one for the deletion commands.

@alexlovelltroy
Copy link
Member

Thinking about existing tools I've used.

--force generally means "do it instead of asking"
--non-interactive generally means "suppress all prompts. No one will see them"

How can we make the flag and the behavior least surprising?

@synackd
Copy link
Collaborator

synackd commented Aug 15, 2025

Using something intuitive while being succinct I think is the goal here. I generally prefer flags that are complementary (e.g. --yes/--no) so that the opposite of one flag is predictable. Also, whatever we decide should be applicable globally to avoid duplication.

With scriptability in mind, my vision is that --yes would automatically answer yes, effectively forcing the action to happen without prompting, while --no would non-interactively answer no if prompted, causing an error so that the script doesn't hang indefinitely at a prompt and errs immediately (e.g. if the user intends to modify an existing config file but wants to detect if one does not already exist, since the prompt appears only if the config file does not already exist). The problem I see with --non-interactive or --force is that this second behavior is not enforced.

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.

4 participants