[dftimewolf] This PR contains a big revamp of dftimeflow's code.

[dftimewolf] Big revamp of dftimeflow's code.

Notable changes:
 * Removed all of dftimewolf's old code
 * Import of modules and recipes is no longer hardcoded
 * Where modules are imported from is based on paths that can be specified in a configuration file.
 * Code for locating and parsing the configuration file is located in internals.py

[tomchop1, 2017-06-07 12:12:34]

Code updated.

[onager, 2017-06-08 19:06:45]

On 2017/06/07 12:12:34, tomchop1 wrote:
> Code updated.

Please fix the names here - timeflow -> dftimewolf

[tomchop1, 2017-06-09 11:02:02]

Code updated.

[tomchop1, 2017-06-09 11:02:12]

On 2017/06/08 19:06:45, onager wrote:
> On 2017/06/07 12:12:34, tomchop1 wrote:
> > Code updated.
> 
> Please fix the names here - timeflow -> dftimewolf

Done, sorry about that.

[onager, 2017-06-21 15:09:31]

https://codereview.appspot.com/324930043/diff/40001/README.md
File README.md (left):

https://codereview.appspot.com/324930043/diff/40001/README.md#oldcode4
README.md:4: <img
src="https://cloud.githubusercontent.com/assets/13300571/17257013/0065185c-5575-11e6-957d-5e662ec78d8c.png"
width="200"/>
I'm sad that you're getting rid of the logo

https://codereview.appspot.com/324930043/diff/40001/dftimewolf/internals.py
File dftimewolf/internals.py (right):

https://codereview.appspot.com/324930043/diff/40001/dftimewolf/internals.py#n...
dftimewolf/internals.py:88: if not _CONFIG:
Please flatten this, it's very heavily nested, and some guard clauses would help
make this cleaner.

I also have strong reservations about using a global like this...

https://codereview.appspot.com/324930043/diff/40001/dftimewolf/lib/collectors...
File dftimewolf/lib/collectors/filesystem.py (right):

https://codereview.appspot.com/324930043/diff/40001/dftimewolf/lib/collectors...
dftimewolf/lib/collectors/filesystem.py:29: self.output = None
output looks to be missing from the class docstring. Please fix.

[tomchop1, 2017-06-22 14:00:06]

Code updated.

[tomchop1, 2017-06-22 14:00:42]

https://codereview.appspot.com/324930043/diff/40001/dftimewolf/internals.py
File dftimewolf/internals.py (right):

https://codereview.appspot.com/324930043/diff/40001/dftimewolf/internals.py#n...
dftimewolf/internals.py:88: if not _CONFIG:
On 2017/06/21 15:09:31, onager wrote:
> Please flatten this, it's very heavily nested, and some guard clauses would
help
> make this cleaner.
> 
> I also have strong reservations about using a global like this...

Regarding the global, we could write a new singleton class, but it will add
several lines of code for a semantically similar function. We could also have
get_config() search for and read files every time, but that's not very
effective.

[someguyiknow, 2017-06-28 02:00:26]

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/recipes/lo...
File dftimewolf/cli/recipes/local_plaso.py (right):

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/recipes/lo...
dftimewolf/cli/recipes/local_plaso.py:49: ('--incident_id', 'Incident ID (used
for Timesketch descrption)'),
what makes -- different?

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/recipes/lo...
dftimewolf/cli/recipes/local_plaso.py:49: ('--incident_id', 'Incident ID (used
for Timesketch descrption)'),
description

[tomchop1, 2017-06-28 11:20:54]

On 2017/06/28 02:00:26, someguyiknow wrote:
>
https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/recipes/lo...
> File dftimewolf/cli/recipes/local_plaso.py (right):
> 
>
https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/recipes/lo...
> dftimewolf/cli/recipes/local_plaso.py:49: ('--incident_id', 'Incident ID (used
> for Timesketch descrption)'),
> what makes -- different?

-- makes it an optional argument; the CLI won't fail if it is not provided.

> 
>
https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/recipes/lo...
> dftimewolf/cli/recipes/local_plaso.py:49: ('--incident_id', 'Incident ID (used
> for Timesketch descrption)'),
> description

Thanks.

[onager, 2017-07-03 14:15:42]

A couple more things - more thorough review coming soon

https://codereview.appspot.com/324930043/diff/60001/README
File README (right):

https://codereview.appspot.com/324930043/diff/60001/README#newcode1
README:1: ## DFTimewolf
Move this to readme.md (it's in markdown anyway)

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
File dftimewolf/cli/dftimewolf_recipes.py (right):

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:3: dftimewolf_recipes aims to replace
dftimewolf_cli. It uses recipes defined in
Drop this first sentence, it's not necessary what this is replacing.

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:40: from dftimewolf.internals import
import_modules
Please don't import functions like this:
https://google.github.io/styleguide/pyguide.html#Imports

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:48: def import_args_from_cli(elt, args):
What's an elt? Please rename this to something meaningful.

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:57: args: argparse parse_args() object
Is this the function object, or an argparse.Namespace?

[tomchop1, 2017-07-03 14:51:56]

Code updated.

[tomchop1, 2017-07-04 09:10:08]

https://codereview.appspot.com/324930043/diff/60001/README
File README (right):

https://codereview.appspot.com/324930043/diff/60001/README#newcode1
README:1: ## DFTimewolf
On 2017/07/03 14:15:41, onager wrote:
> Move this to readme.md (it's in markdown anyway)

Done.

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
File dftimewolf/cli/dftimewolf_recipes.py (right):

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:3: dftimewolf_recipes aims to replace
dftimewolf_cli. It uses recipes defined in
On 2017/07/03 14:15:42, onager wrote:
> Drop this first sentence, it's not necessary what this is replacing.

Done.

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:40: from dftimewolf.internals import
import_modules
On 2017/07/03 14:15:42, onager wrote:
> Please don't import functions like this:
> https://google.github.io/styleguide/pyguide.html#Imports

As discussed, will change in future CL.

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:48: def import_args_from_cli(elt, args):
On 2017/07/03 14:15:42, onager wrote:
> What's an elt? Please rename this to something meaningful.

It's a value in a key: value dict. Renamed to value, but this will probably
change after we refactor the config into a Class instead of a dict.

https://codereview.appspot.com/324930043/diff/60001/dftimewolf/cli/dftimewolf...
dftimewolf/cli/dftimewolf_recipes.py:57: args: argparse parse_args() object
On 2017/07/03 14:15:42, onager wrote:
> Is this the function object, or an argparse.Namespace?

Done.

[tomchop1, 2017-07-04 09:10:08]

Code updated.

[onager, 2017-07-04 09:13:55]

As discussed, let's get this committed and clean up afterwards.

LGTM

[tomchop1, 2017-07-04 11:25:31]

Changes have been merged with master branch. To close the review and clean up
the feature branch you can run: python ./utils/review.py close timeflow