[plaso] Moved queue consumer out of analysis plugin #366

[plaso] Moved queue consumer out of analysis plugin #366

Depends on:
https://codereview.appspot.com/302250043/

[Joachim Metz, 2016-07-23 18:35:22]

Code updated.

[onager, 2016-07-24 14:30:56]

Reviewing out of order, sorry.

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/interface.py
File plaso/analysis/interface.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/interface....
plaso/analysis/interface.py:29: """Class that implements the analysis plugin
interface."""
-implements +defines

The plugins implement the interface.

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/manager.py
File plaso/analysis/manager.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/manager.py...
plaso/analysis/manager.py:95: A tuple that contains the uniquely identifying
name of the plugin
Please fix docstring.

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py
File plaso/frontend/psort.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py#n...
plaso/frontend/psort.py:238: use_time_slicer (Optional[bool]): True if the 'time
slicer' should be
Why does this arg exist? Surely we could just check if time_slice is not None...

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py#n...
plaso/frontend/psort.py:345: output_event_queue =
multi_process_queue.MultiProcessingQueue(timeout=5)
Please remove hard coded timeout.

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py#n...
plaso/frontend/psort.py:489: storage_file_path (str): path of the storage file.
-of +to

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
File plaso/multi_processing/analysis_process.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:34: knowledge_base (KnowledgeBase):
knowledge base which contains
-"knowledge base which"

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:36: plugin: the plugin running in the
process (instance of AnalysisProcess).
-the

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:81: if isinstance(task,
plaso_queue.QueueAbort):
event, not task

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:101: analysis_report.time_compiled =
timelib.Timestamp.GetNow()
This should be done in the mediator, not the process.

https://codereview.appspot.com/302260043/diff/20001/plaso/parsers/manager.py
File plaso/parsers/manager.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/parsers/manager.py#...
plaso/parsers/manager.py:235: dict[str,BaseParser]: parsers per name.
+ space

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/browser_se...
File tests/analysis/browser_search.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/browser_se...
tests/analysis/browser_search.py:27: analysis_plugin =
browser_search.BrowserSearchPlugin()
just plugin

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/chrome_ext...
File tests/analysis/chrome_extension.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/chrome_ext...
tests/analysis/chrome_extension.py:98: analysis_plugin =
MockChromeExtensionPlugin()
plugin

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/file_hashe...
File tests/analysis/file_hashes.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/file_hashe...
tests/analysis/file_hashes.py:36: def testExamineEvent(self):
This also tests CompileReport - please rename

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/nsrlsvr.py
File tests/analysis/nsrlsvr.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/nsrlsvr.py...
tests/analysis/nsrlsvr.py:90: def testExamineEvent(self):
This also test the compilereport method, please rename

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/unique_dom...
File tests/analysis/unique_domains_visited.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/unique_dom...
tests/analysis/unique_domains_visited.py:36: def testExamineEvent(self):
This also tests the compilereport method, please rename. This goes for the other
similar methods too.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/viper.py
File tests/analysis/viper.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/viper.py#n...
tests/analysis/viper.py:42: """Creates a mock method to simulate a Viper API
request.
This doesn't create a mock method.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/viper.py#n...
tests/analysis/viper.py:46: data (dict[str,str]): simulated form data for the
Viper API request.
+ space, and not all the values are strings (see id, below).

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/virustotal.py
File tests/analysis/virustotal.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/virustotal...
tests/analysis/virustotal.py:43: """Creates a mock method to simulate a
VirusTotal API request.
Doesn't create a method

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/virustotal...
tests/analysis/virustotal.py:47: params (dict[str,str]): HTTP parameters for the
VirusTotal API request.
+ space

[Joachim Metz, 2016-07-24 15:22:17]

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/interface.py
File plaso/analysis/interface.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/interface....
plaso/analysis/interface.py:29: """Class that implements the analysis plugin
interface."""
On 2016/07/24 14:30:55, onager wrote:
> -implements +defines
> 
> The plugins implement the interface.

Done.

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/manager.py
File plaso/analysis/manager.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/analysis/manager.py...
plaso/analysis/manager.py:95: A tuple that contains the uniquely identifying
name of the plugin
On 2016/07/24 14:30:55, onager wrote:
> Please fix docstring.

Done.

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py
File plaso/frontend/psort.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py#n...
plaso/frontend/psort.py:238: use_time_slicer (Optional[bool]): True if the 'time
slicer' should be
I've added a reminder in https://github.com/log2timeline/plaso/issues/366. Out
of scope for this CL.

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py#n...
plaso/frontend/psort.py:345: output_event_queue =
multi_process_queue.MultiProcessingQueue(timeout=5)
replaced by _QUEUE_TIMEOUT

https://codereview.appspot.com/302260043/diff/20001/plaso/frontend/psort.py#n...
plaso/frontend/psort.py:489: storage_file_path (str): path of the storage file.
On 2016/07/24 14:30:55, onager wrote:
> -of +to

Done.

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
File plaso/multi_processing/analysis_process.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:34: knowledge_base (KnowledgeBase):
knowledge base which contains
On 2016/07/24 14:30:55, onager wrote:
> -"knowledge base which"

Done.

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:36: plugin: the plugin running in the
process (instance of AnalysisProcess).
On 2016/07/24 14:30:55, onager wrote:
> -the

Done.

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:81: if isinstance(task,
plaso_queue.QueueAbort):
On 2016/07/24 14:30:55, onager wrote:
> event, not task

Done.

https://codereview.appspot.com/302260043/diff/20001/plaso/multi_processing/an...
plaso/multi_processing/analysis_process.py:101: analysis_report.time_compiled =
timelib.Timestamp.GetNow()
This will be moved after deprecating analysis_report_queue

https://codereview.appspot.com/302260043/diff/20001/plaso/parsers/manager.py
File plaso/parsers/manager.py (right):

https://codereview.appspot.com/302260043/diff/20001/plaso/parsers/manager.py#...
plaso/parsers/manager.py:235: dict[str,BaseParser]: parsers per name.
On 2016/07/24 14:30:55, onager wrote:
> + space

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/browser_se...
File tests/analysis/browser_search.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/browser_se...
tests/analysis/browser_search.py:27: analysis_plugin =
browser_search.BrowserSearchPlugin()
On 2016/07/24 14:30:55, onager wrote:
> just plugin

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/chrome_ext...
File tests/analysis/chrome_extension.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/chrome_ext...
tests/analysis/chrome_extension.py:98: analysis_plugin =
MockChromeExtensionPlugin()
On 2016/07/24 14:30:55, onager wrote:
> plugin

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/file_hashe...
File tests/analysis/file_hashes.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/file_hashe...
tests/analysis/file_hashes.py:36: def testExamineEvent(self):
On 2016/07/24 14:30:55, onager wrote:
> This also tests CompileReport - please rename

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/nsrlsvr.py
File tests/analysis/nsrlsvr.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/nsrlsvr.py...
tests/analysis/nsrlsvr.py:90: def testExamineEvent(self):
On 2016/07/24 14:30:55, onager wrote:
> This also test the compilereport method, please rename

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/unique_dom...
File tests/analysis/unique_domains_visited.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/unique_dom...
tests/analysis/unique_domains_visited.py:36: def testExamineEvent(self):
On 2016/07/24 14:30:56, onager wrote:
> This also tests the compilereport method, please rename. This goes for the
other
> similar methods too.

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/viper.py
File tests/analysis/viper.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/viper.py#n...
tests/analysis/viper.py:42: """Creates a mock method to simulate a Viper API
request.
On 2016/07/24 14:30:56, onager wrote:
> This doesn't create a mock method.

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/viper.py#n...
tests/analysis/viper.py:46: data (dict[str,str]): simulated form data for the
Viper API request.
On 2016/07/24 14:30:56, onager wrote:
> + space, and not all the values are strings (see id, below).

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/virustotal.py
File tests/analysis/virustotal.py (right):

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/virustotal...
tests/analysis/virustotal.py:43: """Creates a mock method to simulate a
VirusTotal API request.
On 2016/07/24 14:30:56, onager wrote:
> Doesn't create a method

Done.

https://codereview.appspot.com/302260043/diff/20001/tests/analysis/virustotal...
tests/analysis/virustotal.py:47: params (dict[str,str]): HTTP parameters for the
VirusTotal API request.
On 2016/07/24 14:30:56, onager wrote:
> + space

Done.

[Joachim Metz, 2016-07-24 15:28:12]

Code updated.

[onager, 2016-07-25 12:03:20]

Docstring nites, LGTM

https://codereview.appspot.com/302260043/diff/40001/tests/analysis/viper.py
File tests/analysis/viper.py (right):

https://codereview.appspot.com/302260043/diff/40001/tests/analysis/viper.py#n...
tests/analysis/viper.py:42: """Mock funtion to simulate a Viper API request.
function

And this should follow the "active style" of the other docstrings - "Mocks a
call to..." or "Simulates a POST to..."

https://codereview.appspot.com/302260043/diff/40001/tests/analysis/virustotal.py
File tests/analysis/virustotal.py (right):

https://codereview.appspot.com/302260043/diff/40001/tests/analysis/virustotal...
tests/analysis/virustotal.py:43: """Mock function to simulate a VirusTotal API
request.
As before, re: active style

[Joachim Metz, 2016-07-25 16:16:17]

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 psort2