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

Handling when a user does not answer one of the questions #189

Merged
merged 25 commits into from
Jul 24, 2024

Conversation

yaleman
Copy link
Contributor

@yaleman yaleman commented Jul 13, 2024

If you ran contentctl new and hit ctrl-c it'd end up throwing a slightly confusing error:

$ contentctl new
? enter detection name Powershell Encoded Command

Cancelled by user

Verbose error logging is ENABLED.
The entire stack trace has been provided below (please include it if filing a bug report):

Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/contentctl/contentctl.py", line 205, in main
    new_func(config)
  File "/usr/local/lib/python3.12/site-packages/contentctl/contentctl.py", line 93, in new_func
    NewContent().execute(config)
  File "/usr/local/lib/python3.12/site-packages/contentctl/actions/new_content.py", line 97, in execute
    content_dict = self.buildDetection()
                   ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/contentctl/actions/new_content.py", line 21, in buildDetection
    answers['name'] = answers['detection_name']
                      ~~~~~~~^^^^^^^^^^^^^^^^^^
KeyError: 'detection_name'

@pyth0n1c
Copy link
Contributor

Hi @yaleman, thank you for the PR!
I have looked at the questionary codebase, and when a keyboard interrupt is captured during a questionary.prompt() call, there is an inconsistency between the documentation (should return None) and codebase for questionary (actually returns {}).
I have opened an issue here: tmbo/questionary#385

I would propose that instead, we do two things:

  1. Use a more descriptive keyboard interrupt message, which is supported by questionary, such as: answers = questionary.prompt(questions,kbi_msg="User did not answer all of the prompt questions. Exiting...")
  2. Check the return value or the questionary.prompt() and, if it is None (or {}, depending on the response from the questionary team) we raise an Exception which is then handled properly by the contentctl.py's catch-all exception handler.

@yaleman
Copy link
Contributor Author

yaleman commented Jul 17, 2024

That'd work as well 😄 Apologies for the epic reformat, my defaults in VS code are a little different, can clean that up when I go back to make changes.

We don't even have to check for {} / None, it can just be if not answers which does the same.

The catch-nothing exception sounds sensible, if I should create a new one please suggest a name, or one that will be handled (because the KeyError wasn't 😄 )

@yaleman yaleman force-pushed the handle-no-answer branch from 81274d2 to 14d02a2 Compare July 19, 2024 04:37
@yaleman yaleman force-pushed the handle-no-answer branch from 14d02a2 to 858a050 Compare July 19, 2024 04:38
@yaleman
Copy link
Contributor Author

yaleman commented Jul 19, 2024

Cleaned up the commit and made the handling a little cleaner (turns out, vs-code has a "save without formatting option").

yaleman and others added 20 commits July 19, 2024 14:45
…les. Improve the error message generated when a file that does not end in csv, yml, or mlmodel has been changed in the lookups directory.
ensure that each row has at least
one row, so that fields/headings
can be extracted, and that each
row has the proper number of
fields in it. Report verbose errors
for each row if there is an issue.
allowed in the lookups folder. A new function is introduced
that improves checking on what file extensions are
allowed in any folder which should supercede most
usage of the Utils.get_all_yml_files_from_directory
function in the future.
@pyth0n1c
Copy link
Contributor

I am still waiting on some feedback on the Issue I created in the questionary repo: #189

I have decided we will merge this in, as your updated changes handle both cases {} and None.
I will keep an eye on the linked questionary issue above.

I have updated this to go into v4.2 which we expect to release soon rather than directly into main.

@pyth0n1c pyth0n1c changed the base branch from main to release_v4.2.0 July 23, 2024 23:57
@pyth0n1c pyth0n1c merged commit 741c3b7 into splunk:release_v4.2.0 Jul 24, 2024
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