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

Added a batch option #150

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft

Added a batch option #150

wants to merge 3 commits into from

Conversation

lap1nou
Copy link

@lap1nou lap1nou commented Mar 24, 2023

Hello,

Description

This PR adds a -b / --batch option to answer all Confirm() call by the default value, this can be useful in case of unattending installation, for example, if Exegol is deployed at scale using Vagrant / SSCM / ...

Related issues

N / A

Point of attention

I only tested a classic exegol install osint -b, I hope this won't break anything.

@ShutdownRepo ShutdownRepo requested a review from Dramelac March 26, 2023 17:03
@ShutdownRepo ShutdownRepo changed the base branch from master to dev March 26, 2023 17:04
@ShutdownRepo
Copy link
Member

Fixed the target branch, can you resolve the conflicts?
I'll let @Dramelac then review the PR, as there's somethings very similar in his todolist somewhere if I remember correctly

@Dramelac
Copy link
Member

Dramelac commented Apr 3, 2023

Hello @lap1nou,

Thank you for your PR, this feature is on our roadmap as @ShutdownRepo mentioned but hasn't been completed yet because of its complexity.
Exegol is very flexible to allow a seamless experience for the user and the Confirm function isn't the only one concern by this feature to fully enable a non-interactive mode.

I will keep your PR on hold for now because there is a lot missing. But, if you have some time to go further i can go into more details. Otherwise i will handle this subject later.

@lap1nou
Copy link
Author

lap1nou commented Apr 7, 2023

Hey @Dramelac, @ShutdownRepo, thank you for your answers.

If you have other examples of things to change I can try to dig into the subject.

Regards.

@Dramelac
Copy link
Member

To give you more insight, Exegol has been thought for its UX priority, when an important information is missing, an interaction with the user will be requested. I had started a draft of ParametersManager().interactive_mode to disable the interactive behavior and identify the parameters to be considered critical to block the execution of the program immediately.
I didn't have too much time to think about the system until the end but currently we don't really use the "required": False of the groupArgs of each action, it can become a relevant tool in this feature.

Otherwise here are some cases where a user interaction will be requested:

  • choose a container
  • choose an image
  • all kind of Y/N confirmations (delete, multi-select etc)
  • configuration changes according to some options (however, the default choice proposed to the user should not necessarily be the one to apply in non-interactive mode, for example when a VPN is configured, exegol wrapper will propose to disable the network mode host, in non-interactive mode it is not necessary to follow the default choice which is to change otherwise it would block this option)
  • the choice of image build profile
  • the choice of git branch for checkout
  • and probably other type i'm forgetting here...

In each case we have to choose if we want to disable or if we want to take an arbitrary choice. In cases where a CLI option is possible, why not make a critical parameter if its use is justified for example the choice of build profile is mandatory but the change of branch can be ignored if there are no parameters

Here are some issues / thoughts I had on the subject, if you want to take the subject and start to do things let me know so I do not launch equivalent work alongside.

@Dramelac Dramelac added the waiting for additional changes Further changes are requested label Apr 28, 2023
Copy link
Member

@Dramelac Dramelac left a comment

Choose a reason for hiding this comment

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

In this state it's not enough to be merge into the wrapper

@Dramelac Dramelac marked this pull request as draft October 25, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for additional changes Further changes are requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants