FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Added support to process empty records by vmalaviya-splunk · Pull Request #402 · splunk/splunk-sdk-python · GitHub

Repository navigation

Added support to process empty records - #402

Merged
ashah-splunk merged 13 commits into
developfrom
DVPL-10069
Oct 27, 2021
Merged

ashah-splunk merged 13 commits into
developfrom
DVPL-10069

Conversation

Copy link
Copy Markdown
Contributor

No description provided.

vmalaviya-splunk changed the title Added support to process empty records for CSC. Added support to process empty records Oct 4, 2021

fantavlik left a comment •
edited by vmalaviya-splunk
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I have some additional suggestions - we talked about adding tests, see below.

More intuitive name

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

Generating commands

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)

Allow empty by default

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.



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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add a docstring below for the new arg

:param ofile: Output data file.
:type ofile: file

:param allow_empty_list: Allow empty results

fantavlik Oct 7, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

fantavlik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Added suggestion for a unit test and code comment but overall this looks good - feel free to merge when those are added

fantavlik self-requested a review October 26, 2021 18:17

fantavlik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for all the updates, looks great 🚀

vmalaviya-splunk mentioned this pull request Nov 2, 2021
vmalaviya-splunk deleted the DVPL-10069 branch May 2, 2022 12:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL