Adds Splunk interfaces

[markkasaboski]

No description provided.

[markkasaboski]

@aviau I see you approved my draft. I was still working on the refactoring portion of it. Can you have a look now and give me your thoughts before I keep moving forward with the tests? Thanks.

• Refactored the cron_job_ingest_events.py code into a class
  -- This cleans things up by not having to pass dependencies all the way down through every function.
• Refactored test_ingest_events.py to use fake objects instead of mocks.

Not done with testing yet. It's mostly there, but opening for review now.

[aviau]

Can you have a look now and give me your thoughts before I keep moving forward with the tests?

Sure! My first comment would be that this PR is pretty big already. There are multiple refactors at the same time:

• Using interfaces
• Creating a big FlareEventIngestor class

I think the simplicify of the previous design was nice. Now, the FlareEventIngestor will be hard to test atomically because it has a lot of dependencies. For example, testing get_storage_password_value now requires creating a whole FlareEventIngestor, but it only needs one of its properties.

Instead of creating this big class, you could create something like this:

def get_storage_password_value(
   storage: StoragePasswords,
   ...
 ):
    ...

When you attach a method to a class, this makes the method impossible to test without the class. You should question: does it really need to be attached to the class?

[markkasaboski]

Can you have a look now and give me your thoughts before I keep moving forward with the tests?

Sure! My first comment would be that this PR is pretty big already. There are multiple refactors at the same time:

• Using interfaces
• Creating a big FlareEventIngestor class

I think the simplicify of the previous design was nice. Now, the FlareEventIngestor will be hard to test atomically because it has a lot of dependencies. For example, testing get_storage_password_value now requires creating a whole FlareEventIngestor, but it only needs one of its properties.

Instead of creating this big class, you could create something like this:

def get_storage_password_value(
   storage: StoragePasswords,
   ...
 ):
    ...

When you attach a method to a class, this makes the method impossible to test without the class. You should question: does it really need to be attached to the class?

@aviau

The problem I see with the function above is that every function that we have in this file has to be passed its dependency which is either storage_passwords: StoragePasswords or kvstore: KVStoreCollections, which makes it rather messy. If it's in a class, every method can simply reference the class property where the dependency reside.

I dunno what this boils down to. Is this just a matter of personal preference or best practice? I can trash the class and go back.

Wanna/have time to pair up on it?

[aviau]

If it's in a class, every method can simply reference the class property where the dependency reside.

So you can do this:

class BigClass:
    def do_something(arg: str) -> None:
       # This just re-routes to the detached function, and is more "convenient" to use.
        return do_something(dependency=self.dependency, arg=arg)


def do_something(dependency, arg) -> None:
  # most of the implementation is here, and it has minimal dependencies
  ...

This way, you can test do_someting easily, without creating BigClass, and you stil get the "convenience" when you use BigClass.

[TyMarc]

LGTM