| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
I have some additional suggestions - we talked about adding tests, see below.
I think allow_empty_list probably doesn't mean anything to our users so maybe something more along the lines of allow_empty_chunks or allow_empty_records
I was also thinking about generating commands, they have their own implementation for _execute_chunk_v2 that obviously assumes that no events are present because that's how generating commands work. So we should probably enforce that by not supplying to parameter in the process method for generating commands, so adding this to https://github.com/splunk/splunk-sdk-python/blob/master/splunklib/searchcommands/generating_command.py (add adding import sys:
def process(self, argv=sys.argv, ifile=sys.stdin, ofile=sys.stdout):
""" Process data.
:param argv: Command line arguments.
:type argv: list or tuple
:param ifile: Input data file.
:type ifile: file
:param ofile: Output data file.
:type ofile: file
:return: :const:`None`
:rtype: NoneType
"""
# Force allow_empty_list to be true since generating commands assume an empty input
return self.process(argv=argv, ifile=ifile, ofile=ofile, allow_empty_list=True)I was also thinking about actually setting allow_empty_list to True by default for all commands since that seems to match our previous behavior.
Let me know what you think.
Sorry, something went wrong.
|
|
||
|
|
||
| def dispatch(command_class, argv=sys.argv, input_file=sys.stdin, output_file=sys.stdout, module_name=None): | ||
| def dispatch(command_class, argv=sys.argv, input_file=sys.stdin, output_file=sys.stdout, module_name=None, allow_empty_list=False): |
There was a problem hiding this comment.
Add a docstring below for the new arg
Sorry, something went wrong.
| :param ofile: Output data file. | ||
| :type ofile: file | ||
|
|
||
| :param allow_empty_list: Allow empty results |
There was a problem hiding this comment.
Slightly more descriptive:
Allow empty input records for the command, if False an Error will be returned if empty chunk body is encountered when read
Sorry, something went wrong.
There was a problem hiding this comment.
Added suggestion for a unit test and code comment but overall this looks good - feel free to merge when those are added
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for all the updates, looks great 🚀
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
No description provided.