From e14d0e6352efcf78d33c896a425ddc41405acd02 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Wed, 27 Jul 2016 08:15:56 -0500 Subject: [PATCH] Serialize Checkers PluginTypeManager to a dict It seems likely that the multiprocessing module on Windows is not capable of serializing an object with the structure that we have and preserving the attributes we dynamically set on plugins (like the FlakesChecker). To avoid issues like this with all plugins (although we have only found this on Windows with the FlakesChecker), let's try serializing the Checkers PluginTypeManager to a dictionary so that the only object that a Queue is really trying to serialize/deserialize is the FlakesChecker itself. Related to #179 --- src/flake8/checker.py | 48 ++++++++++++++++-------------- src/flake8/exceptions.py | 2 +- src/flake8/plugins/manager.py | 24 +++++++++++++++ src/flake8/processor.py | 4 +-- tests/integration/test_checker.py | 6 +++- tests/unit/test_checker_manager.py | 8 ++++- 6 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/flake8/checker.py b/src/flake8/checker.py index 57fd896..2dc3481 100644 --- a/src/flake8/checker.py +++ b/src/flake8/checker.py @@ -38,21 +38,6 @@ SERIAL_RETRY_ERRNOS = set([ ]) -def _run_checks_from_queue(process_queue, results_queue, statistics_queue): - LOG.info('Running checks in parallel') - try: - for checker in iter(process_queue.get, 'DONE'): - LOG.info('Checking "%s"', checker.filename) - checker.run_checks(results_queue, statistics_queue) - except exceptions.PluginRequestedUnknownParameters as exc: - print(str(exc)) - except Exception as exc: - LOG.error('Unhandled exception occurred') - raise - finally: - results_queue.put('DONE') - - class Manager(object): """Manage the parallelism and checker instances for each plugin and file. @@ -284,8 +269,9 @@ class Manager(object): file_exists = os.path.exists(filename) return (file_exists and matches_filename_patterns) or is_stdin + checks = self.checks.to_dictionary() self.checkers = [ - FileChecker(filename, self.checks, self.options) + FileChecker(filename, checks, self.options) for argument in paths for filename in utils.filenames_from(argument, self.is_path_excluded) @@ -405,7 +391,7 @@ class FileChecker(object): :param checks: The plugins registered to check the file. :type checks: - flake8.plugins.manager.Checkers + dict :param options: Parsed option values from config and command-line. :type options: @@ -458,14 +444,17 @@ class FileChecker(object): """Run the check in a single plugin.""" LOG.debug('Running %r with %r', plugin, arguments) try: - self.processor.keyword_arguments_for(plugin.parameters, arguments) + self.processor.keyword_arguments_for( + plugin['parameters'], + arguments, + ) except AttributeError as ae: LOG.error('Plugin requested unknown parameters.') raise exceptions.PluginRequestedUnknownParameters( plugin=plugin, exception=ae, ) - return plugin.execute(**arguments) + return plugin['plugin'](**arguments) def run_ast_checks(self): """Run all checks expecting an abstract syntax tree.""" @@ -484,7 +473,7 @@ class FileChecker(object): (exc_type.__name__, exception.args[0])) return - for plugin in self.checks.ast_plugins: + for plugin in self.checks['ast_plugins']: checker = self.run_check(plugin, tree=ast) # If the plugin uses a class, call the run method of it, otherwise # the call should return something iterable itself @@ -509,7 +498,7 @@ class FileChecker(object): LOG.debug('Logical line: "%s"', logical_line.rstrip()) - for plugin in self.checks.logical_line_plugins: + for plugin in self.checks['logical_line_plugins']: self.processor.update_checker_state_for(plugin) results = self.run_check(plugin, logical_line=logical_line) or () for offset, text in results: @@ -526,7 +515,7 @@ class FileChecker(object): def run_physical_checks(self, physical_line, override_error_line=None): """Run all checks for a given physical line.""" - for plugin in self.checks.physical_line_plugins: + for plugin in self.checks['physical_line_plugins']: self.processor.update_checker_state_for(plugin) result = self.run_check(plugin, physical_line=physical_line) if result is not None: @@ -636,6 +625,21 @@ class FileChecker(object): override_error_line=token[4]) +def _run_checks_from_queue(process_queue, results_queue, statistics_queue): + LOG.info('Running checks in parallel') + try: + for checker in iter(process_queue.get, 'DONE'): + LOG.info('Checking "%s"', checker.filename) + checker.run_checks(results_queue, statistics_queue) + except exceptions.PluginRequestedUnknownParameters as exc: + print(str(exc)) + except Exception as exc: + LOG.error('Unhandled exception occurred') + raise + finally: + results_queue.put('DONE') + + def find_offset(offset, mapping): """Find the offset tuple for a single offset.""" if isinstance(offset, tuple): diff --git a/src/flake8/exceptions.py b/src/flake8/exceptions.py index 71738b2..1ba5966 100644 --- a/src/flake8/exceptions.py +++ b/src/flake8/exceptions.py @@ -65,7 +65,7 @@ class PluginRequestedUnknownParameters(Flake8Exception): def __str__(self): """Format our exception message.""" - return self.FORMAT % {'name': self.plugin.plugin_name, + return self.FORMAT % {'name': self.plugin['plugin_name'], 'exc': self.original_exception} diff --git a/src/flake8/plugins/manager.py b/src/flake8/plugins/manager.py index 28f38a6..a5f1c3b 100644 --- a/src/flake8/plugins/manager.py +++ b/src/flake8/plugins/manager.py @@ -49,6 +49,16 @@ class Plugin(object): self.name, self.entry_point ) + def to_dictionary(self): + """Convert this plugin to a dictionary.""" + return { + 'name': self.name, + 'parameters': self.parameters, + 'parameter_names': self.parameter_names, + 'plugin': self.plugin, + 'plugin_name': self.plugin_name, + } + def is_in_a_group(self): """Determine if this plugin is in a group. @@ -433,6 +443,20 @@ class Checkers(PluginTypeManager): if argument_name == plugin.parameter_names[0]: yield plugin + def to_dictionary(self): + """Return a dictionary of AST and line-based plugins.""" + return { + 'ast_plugins': [ + plugin.to_dictionary() for plugin in self.ast_plugins + ], + 'logical_line_plugins': [ + plugin.to_dictionary() for plugin in self.logical_line_plugins + ], + 'physical_line_plugins': [ + plugin.to_dictionary() for plugin in self.physical_line_plugins + ], + } + def register_options(self, optmanager): """Register all of the checkers' options to the OptionManager. diff --git a/src/flake8/processor.py b/src/flake8/processor.py index 1d0e3a4..8f72e65 100644 --- a/src/flake8/processor.py +++ b/src/flake8/processor.py @@ -129,9 +129,9 @@ class FileProcessor(object): def update_checker_state_for(self, plugin): """Update the checker_state attribute for the plugin.""" - if 'checker_state' in plugin.parameters: + if 'checker_state' in plugin['parameters']: self.checker_state = self._checker_states.setdefault( - plugin.name, {} + plugin['name'], {} ) def next_logical_line(self): diff --git a/tests/integration/test_checker.py b/tests/integration/test_checker.py index 5c013b6..1b8a3c5 100644 --- a/tests/integration/test_checker.py +++ b/tests/integration/test_checker.py @@ -63,7 +63,11 @@ def test_handle_file_plugins(plugin_target): # Prevent it from reading lines from stdin or somewhere else with mock.patch('flake8.processor.FileProcessor.read_lines', return_value=['Line 1']): - file_checker = checker.FileChecker('-', checks, mock.MagicMock()) + file_checker = checker.FileChecker( + '-', + checks.to_dictionary(), + mock.MagicMock() + ) # Do not actually build an AST file_checker.processor.build_ast = lambda: True diff --git a/tests/unit/test_checker_manager.py b/tests/unit/test_checker_manager.py index 7e9c5b1..e5562ef 100644 --- a/tests/unit/test_checker_manager.py +++ b/tests/unit/test_checker_manager.py @@ -47,8 +47,14 @@ def test_make_checkers(): """Verify that we create a list of FileChecker instances.""" style_guide = style_guide_mock() files = ['file1', 'file2'] + checkplugins = mock.Mock() + checkplugins.to_dictionary.return_value = { + 'ast_plugins': [], + 'logical_line_plugins': [], + 'physical_line_plugins': [], + } with mock.patch('flake8.checker.multiprocessing', None): - manager = checker.Manager(style_guide, files, []) + manager = checker.Manager(style_guide, files, checkplugins) with mock.patch('flake8.utils.filenames_from') as filenames_from: filenames_from.side_effect = [['file1'], ['file2']]