Fix Logic, Improve Test Coverage, and Resilience

[johannes-gehrs]

• When using an incorrect date pattern for the index format, the comparison of date stings meant that incorrect results could occur. This has lead to unintended deletion of data in our environment. This PR fixes this by comparing Python Date objects instead.
• It looks to me like the use of <= in the comparison meant that one more day than intended was deleted. See code.
• The decision logic is factored out into a pure function which is easily testable.
• Some unit tests have been added.
• In order to be able to import the module from the unit test file, I needed to rename the file. Module names with dashes cannot be easily imported.

[giuliocalzolari]

Hi @johannes-gehrs thanks for your PR.
can I ask you why do you use a local function https://github.com/cloudreach/aws-lambda-es-cleanup/pull/37/files#diff-aedd2d570ffd28f6b0994ddd7fd88cddR155?

the method that you implement looks great and also allow to implement the unittest, but could we create a new method of the main class ES_Cleanup ?
is there any specific reason why you use a local function should_delete?

[johannes-gehrs]

If it is a method then you always need to mock "god object" ES_Cleanup in unit tests.

It is doable and if you insist I could refactor to that but I personally like normal functions, especially when we generally think about "pure logic" like we do here. 😄

[johannes-gehrs]

If you are uncomfortable with the higher order function (not sure if that is the case) it would be relatively easy to refactor this to use a normal Decider class which instances get initialized with the values we now pass to delete_decider. And then we can call a method decide on the class instance.

[giuliocalzolari]

having a class Decider would be awesome!

[giuliocalzolari]

@johannes-gehrs did you have some time to create the Decider class?

[johannes-gehrs]

Sorry for the long delay. I have refactored to a class. Everything else should have stayed the same but do you have a way to test before merging and releasing?

[giuliocalzolari]

Yes I have a script to deploy aws es and create multiple indexes to test it. Right now I deploy only for big changes.

[johannes-gehrs]

Let's get this merged?

[giuliocalzolari]

@johannes-gehrs I found some minor issue in the code, I've opened a PR on your repo
can you have a look?
johannes-gehrs#1

[giuliocalzolari]

@johannes-gehrs did you have a chance to review it?

[giuliocalzolari]

@johannes-gehrs

[giuliocalzolari]

can you please merge my minor changes
johannes-gehrs#1

[giuliocalzolari]

can you please merge my minor changes
johannes-gehrs#1

[giuliocalzolari]

can you please merge my minor changes
johannes-gehrs#1

[giuliocalzolari]

I've done some minor change
here johannes-gehrs#1
can you merge you your branch?