[plaso] Code clean up of tools

[plaso] Code clean up of tools

[onager, 2017-10-13 20:41:59]

On 2017/10/13 12:17:14, Joachim Metz wrote:

Note this CL conflicts with https://codereview.appspot.com/330540043/ and
https://codereview.appspot.com/333850043/

[onager, 2017-10-13 20:45:02]

https://codereview.appspot.com/327670043/diff/1/plaso/cli/log2timeline_tool.py
File plaso/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/327670043/diff/1/plaso/cli/log2timeline_tool.p...
plaso/cli/log2timeline_tool.py:46: tool_options.OutputModuleOptions,
Why does log2timeline need to do anything with output modules? It shouldn't need
any of these options.

[Joachim Metz, 2017-10-14 08:14:11]

https://codereview.appspot.com/327670043/diff/1/plaso/cli/log2timeline_tool.py
File plaso/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/327670043/diff/1/plaso/cli/log2timeline_tool.p...
plaso/cli/log2timeline_tool.py:46: tool_options.OutputModuleOptions,
good catch, seems to be regression due to merge

[Joachim Metz, 2017-10-14 08:18:01]

Code updated.

[aaronp, 2017-10-15 05:50:06]

Looks like Onager already reviewed this, let me know if you also want my review
for any reason.

[onager, 2017-10-19 23:28:32]

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (right):

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py#n...
plaso/cli/psort_tool.py:422: self.list_output_modules = self._output_format ==
'list'
Please be careful when rebasing this, it's important that list options are
checked in the correct order.

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (right):

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:42: tool_options.AnalysisPluginOptions,
Please remove this mixin, the functionality isn't enabled.

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:548: self._analysis_plugins =
self._CreateAnalysisPlugins(options)
Analysis plugins don't actually work with psteal (there's no call to
AnalyzeEvents) - please remove.

We should do a separate CL to add the functionality in though.

[Joachim Metz, 2017-10-20 05:06:06]

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (right):

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py#n...
plaso/cli/psort_tool.py:422: self.list_output_modules = self._output_format ==
'list'
what is the correct order and why is there no corresponding comment in the code?

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (right):

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:42: tool_options.AnalysisPluginOptions,
On 2017/10/19 23:28:31, onager wrote:
> Please remove this mixin, the functionality isn't enabled.

Done.

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:548: self._analysis_plugins =
self._CreateAnalysisPlugins(options)
On 2017/10/19 23:28:31, onager wrote:
> Analysis plugins don't actually work with psteal (there's no call to
> AnalyzeEvents) - please remove.
> 
> We should do a separate CL to add the functionality in though.

Acknowledged.

[Joachim Metz, 2017-10-20 05:06:31]

Code updated.

[onager, 2017-10-20 17:02:49]

LGTM

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (right):

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py#n...
plaso/cli/psort_tool.py:422: self.list_output_modules = self._output_format ==
'list'
On 2017/10/20 05:06:06, Joachim Metz wrote:
> what is the correct order and why is there no corresponding comment in the
code?

There is a comment in the updated code ;) "Check output modules after other
listable options..."

[Joachim Metz, 2017-10-20 17:22:10]

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (right):

https://codereview.appspot.com/327670043/diff/20001/plaso/cli/psort_tool.py#n...
plaso/cli/psort_tool.py:422: self.list_output_modules = self._output_format ==
'list'
> "# a required argument will raise."

which required argument? or does it raise with "requires an output file"?

[Joachim Metz, 2017-10-20 17:26:56]

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 tools

[Joachim Metz, 2017-10-20 18:47:18]

https://codereview.appspot.com/327670043/diff/1/plaso/cli/log2timeline_tool.py
File plaso/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/327670043/diff/1/plaso/cli/log2timeline_tool.p...
plaso/cli/log2timeline_tool.py:46: tool_options.OutputModuleOptions,
actually this is legacy behavior, removing anyway.