From 6a1203602f7f7fe8ff5f16b48a8bfbb7191ed5f5 Mon Sep 17 00:00:00 2001 From: Romain Dorgueil Date: Mon, 16 Jul 2018 10:00:28 +0200 Subject: [PATCH 1/3] Minor tweaks about code quality. --- bonobo/config/configurables.py | 2 +- bonobo/config/services.py | 2 -- bonobo/examples/datasets/__main__.py | 2 +- bonobo/execution/contexts/base.py | 5 ++--- bonobo/execution/strategies/executor.py | 6 +++--- bonobo/nodes/io/csv.py | 3 +-- bonobo/plugins/sentry.py | 1 - bonobo/settings.py | 2 +- bonobo/util/environ.py | 1 - 9 files changed, 9 insertions(+), 15 deletions(-) diff --git a/bonobo/config/configurables.py b/bonobo/config/configurables.py index 20ceca5..98e21e7 100644 --- a/bonobo/config/configurables.py +++ b/bonobo/config/configurables.py @@ -72,7 +72,7 @@ class ConfigurableMeta(type): try: import _functools -except: +except ImportError: import functools PartiallyConfigured = functools.partial diff --git a/bonobo/config/services.py b/bonobo/config/services.py index 282d88f..eae3b8d 100644 --- a/bonobo/config/services.py +++ b/bonobo/config/services.py @@ -1,5 +1,3 @@ -import inspect -import pprint import re import threading import types diff --git a/bonobo/examples/datasets/__main__.py b/bonobo/examples/datasets/__main__.py index 91f702d..a62ce33 100644 --- a/bonobo/examples/datasets/__main__.py +++ b/bonobo/examples/datasets/__main__.py @@ -51,7 +51,7 @@ if __name__ == '__main__': s3.head_object( Bucket='bonobo-examples', Key=s3_path ) - except: + except Exception: s3.upload_file( local_path, 'bonobo-examples', diff --git a/bonobo/execution/contexts/base.py b/bonobo/execution/contexts/base.py index b7f07c5..e071266 100644 --- a/bonobo/execution/contexts/base.py +++ b/bonobo/execution/contexts/base.py @@ -1,7 +1,6 @@ import logging import sys from contextlib import contextmanager -from logging import ERROR from bonobo.util import deprecated from bonobo.util.objects import Wrapper, get_name @@ -13,7 +12,7 @@ def recoverable(error_handler): try: yield except Exception as exc: # pylint: disable=broad-except - error_handler(*sys.exc_info(), level=ERROR) + error_handler(*sys.exc_info(), level=logging.ERROR) @contextmanager @@ -21,7 +20,7 @@ def unrecoverable(error_handler): try: yield except Exception as exc: # pylint: disable=broad-except - error_handler(*sys.exc_info(), level=ERROR) + error_handler(*sys.exc_info(), level=logging.ERROR) raise # raise unrecoverableerror from exc ? diff --git a/bonobo/execution/strategies/executor.py b/bonobo/execution/strategies/executor.py index 1e2d45f..e900ce1 100644 --- a/bonobo/execution/strategies/executor.py +++ b/bonobo/execution/strategies/executor.py @@ -29,7 +29,7 @@ class ExecutorStrategy(Strategy): with self.create_executor() as executor: try: context.start(self.get_starter(executor, futures)) - except: + except Exception: logger.critical('Exception caught while starting execution context.', exc_info=sys.exc_info()) while context.alive: @@ -53,14 +53,14 @@ class ExecutorStrategy(Strategy): try: with node: node.loop() - except: + except Exception: logging.getLogger(__name__).critical( 'Critical error in threadpool node starter.', exc_info=sys.exc_info() ) try: futures.append(executor.submit(_runner)) - except: + except Exception: logging.getLogger(__name__).critical('futures.append', exc_info=sys.exc_info()) return starter diff --git a/bonobo/nodes/io/csv.py b/bonobo/nodes/io/csv.py index d7d03fb..7900dca 100644 --- a/bonobo/nodes/io/csv.py +++ b/bonobo/nodes/io/csv.py @@ -1,12 +1,11 @@ import csv -from bonobo.config import Option, use_raw_input, use_context +from bonobo.config import Option, use_context from bonobo.config.options import Method, RenamedOption from bonobo.constants import NOT_MODIFIED from bonobo.nodes.io.base import FileHandler from bonobo.nodes.io.file import FileReader, FileWriter from bonobo.util import ensure_tuple -from bonobo.util.bags import BagType class CsvHandler(FileHandler): diff --git a/bonobo/plugins/sentry.py b/bonobo/plugins/sentry.py index 44799da..4704c0b 100644 --- a/bonobo/plugins/sentry.py +++ b/bonobo/plugins/sentry.py @@ -1,5 +1,4 @@ from bonobo.plugins import Plugin -from raven import Client class SentryPlugin(Plugin): diff --git a/bonobo/settings.py b/bonobo/settings.py index 799ba3d..d87a329 100644 --- a/bonobo/settings.py +++ b/bonobo/settings.py @@ -52,7 +52,7 @@ class Setting: def set(self, value): value = self.formatter(value) if self.formatter else value if self.validator and not self.validator(value): - raise ValidationError('Invalid value {!r} for setting {}.'.format(value, self.name)) + raise ValidationError(self, 'Invalid value {!r} for setting {!r}.'.format(value, self.name)) self.value = value def set_if_true(self, value): diff --git a/bonobo/util/environ.py b/bonobo/util/environ.py index b344d29..980d1db 100644 --- a/bonobo/util/environ.py +++ b/bonobo/util/environ.py @@ -57,7 +57,6 @@ def get_argument_parser(parser=None): :return: """ if parser is None: - import argparse parser = argparse.ArgumentParser() # Store globally to be able to warn the user about the fact he's probably wrong not to pass a parser to From 66451d03bb8effe95582c68e306839ca2eeefa47 Mon Sep 17 00:00:00 2001 From: Romain Dorgueil Date: Sun, 22 Jul 2018 07:34:11 +0200 Subject: [PATCH 2/3] work in progress: working on nodes lifecycle. --- Makefile | 2 +- bonobo/execution/contexts/base.py | 3 +-- bonobo/execution/contexts/graph.py | 18 ++++++++++---- bonobo/execution/contexts/node.py | 24 ++++++++++--------- requirements-dev.txt | 18 +++++++------- requirements-docker.txt | 17 +++++++------ requirements-jupyter.txt | 13 +++++----- requirements-sqlalchemy.txt | 19 +++++++-------- requirements.txt | 17 +++++++------ setup.py | 2 +- .../contexts/test_execution_contexts_graph.py | 6 ++--- ...ode.py => test_execution_contexts_node.py} | 0 12 files changed, 74 insertions(+), 65 deletions(-) rename tests/execution/contexts/{test_node.py => test_execution_contexts_node.py} (100%) diff --git a/Makefile b/Makefile index 29446bc..43c4934 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -# Generated by Medikit 0.6.3 on 2018-06-11. +# Generated by Medikit 0.6.3 on 2018-07-22. # All changes will be overriden. # Edit Projectfile and run “make update” (or “medikit update”) to regenerate. diff --git a/bonobo/execution/contexts/base.py b/bonobo/execution/contexts/base.py index e071266..25cca26 100644 --- a/bonobo/execution/contexts/base.py +++ b/bonobo/execution/contexts/base.py @@ -53,8 +53,7 @@ class Lifecycle: @property def should_loop(self): - # TODO XXX started/stopped? - return not any((self.defunct, self.killed)) + return self.alive and not any((self.defunct, self.killed)) @property def status(self): diff --git a/bonobo/execution/contexts/graph.py b/bonobo/execution/contexts/graph.py index 3c029b5..b2362db 100644 --- a/bonobo/execution/contexts/graph.py +++ b/bonobo/execution/contexts/graph.py @@ -1,14 +1,19 @@ +import logging from functools import partial +from queue import Empty from time import sleep from bonobo.config import create_container from bonobo.constants import BEGIN, END +from bonobo.errors import InactiveReadableError from bonobo.execution import events from bonobo.execution.contexts.base import BaseContext from bonobo.execution.contexts.node import NodeExecutionContext from bonobo.execution.contexts.plugin import PluginExecutionContext from whistle import EventDispatcher +logger = logging.getLogger(__name__) + class GraphExecutionContext(BaseContext): """ @@ -104,11 +109,16 @@ class GraphExecutionContext(BaseContext): sleep(self.TICK_PERIOD) def loop(self): - while self.should_loop: - self.tick() - for node in self.nodes: - if node.should_loop: + nodes = set(node for node in self.nodes if node.should_loop) + while self.should_loop and len(nodes): + self.tick(pause=False) + for node in list(nodes): + try: node.step() + except Empty: + continue + except InactiveReadableError: + nodes.discard(node) def stop(self, stopper=None): super(GraphExecutionContext, self).stop() diff --git a/bonobo/execution/contexts/node.py b/bonobo/execution/contexts/node.py index 07cbf16..0affd29 100644 --- a/bonobo/execution/contexts/node.py +++ b/bonobo/execution/contexts/node.py @@ -120,20 +120,22 @@ class NodeExecutionContext(BaseContext, WithStatistics): break except Empty: sleep(TICK_PERIOD) # XXX: How do we determine this constant? - continue - except ( - NotImplementedError, - UnrecoverableError, - ): - self.fatal(sys.exc_info()) # exit loop - except Exception: # pylint: disable=broad-except - self.error(sys.exc_info()) # does not exit loop - except BaseException: - self.fatal(sys.exc_info()) # exit loop logger.debug('Node loop ends for {!r}.'.format(self)) def step(self): + try: + self._step() + except InactiveReadableError: + raise + except (NotImplementedError, UnrecoverableError, ): + self.fatal(sys.exc_info()) # exit loop + except Exception: # pylint: disable=broad-except + self.error(sys.exc_info()) # does not exit loop + except BaseException: + self.fatal(sys.exc_info()) # exit loop + + def _step(self): """ A single step in the loop. @@ -280,7 +282,7 @@ class NodeExecutionContext(BaseContext, WithStatistics): If Queue raises (like Timeout or Empty), stat won't be changed. """ - input_bag = self.input.get() + input_bag = self.input.get(timeout=0) # Store or check input type if self._input_type is None: diff --git a/requirements-dev.txt b/requirements-dev.txt index 704a481..be0cfb2 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,6 +1,6 @@ -e .[dev] -r requirements.txt -alabaster==0.7.10 +alabaster==0.7.11 arrow==0.12.1 atomicwrites==1.1.5 attrs==18.1.0 @@ -13,7 +13,7 @@ cookiecutter==1.5.1 coverage==4.5.1 docutils==0.14 future==0.16.0 -idna==2.6 +idna==2.7 imagesize==1.0.0 jinja2-time==0.2.0 jinja2==2.10 @@ -22,20 +22,20 @@ more-itertools==4.2.0 packaging==17.1 pluggy==0.6.0 poyo==0.4.1 -py==1.5.3 +py==1.5.4 pygments==2.2.0 pyparsing==2.2.0 pytest-cov==2.5.1 -pytest-timeout==1.2.1 -pytest==3.6.1 +pytest-timeout==1.3.0 +pytest==3.6.3 python-dateutil==2.7.3 -pytz==2018.4 -requests==2.18.4 +pytz==2018.5 +requests==2.19.1 six==1.11.0 snowballstemmer==1.2.1 sphinx-sitemap==0.2 -sphinx==1.7.5 +sphinx==1.7.6 sphinxcontrib-websupport==1.1.0 -urllib3==1.22 +urllib3==1.23 whichcraft==0.4.1 yapf==0.22.0 diff --git a/requirements-docker.txt b/requirements-docker.txt index 08c0bf3..4600a77 100644 --- a/requirements-docker.txt +++ b/requirements-docker.txt @@ -7,24 +7,23 @@ chardet==3.0.4 colorama==0.3.9 docker-pycreds==0.3.0 docker==2.7.0 -fs==2.0.23 -graphviz==0.8.3 -idna==2.6 +fs==2.0.25 +graphviz==0.8.4 +idna==2.7 jinja2==2.10 markupsafe==1.0 mondrian==0.7.0 packaging==17.1 -pbr==4.0.4 +pbr==4.1.1 psutil==5.4.6 pyparsing==2.2.0 python-slugify==1.2.5 -pytz==2018.4 -requests==2.18.4 +pytz==2018.5 +requests==2.19.1 semantic-version==2.6.0 six==1.11.0 -stevedore==1.28.0 -typing==3.6.4 +stevedore==1.29.0 unidecode==1.0.22 -urllib3==1.22 +urllib3==1.23 websocket-client==0.48.0 whistle==1.0.1 diff --git a/requirements-jupyter.txt b/requirements-jupyter.txt index 38aef2a..7f6183f 100644 --- a/requirements-jupyter.txt +++ b/requirements-jupyter.txt @@ -10,7 +10,7 @@ ipykernel==4.8.2 ipython-genutils==0.2.0 ipython==6.4.0 ipywidgets==6.0.1 -jedi==0.12.0 +jedi==0.12.1 jinja2==2.10 jsonschema==2.6.0 jupyter-client==5.2.3 @@ -21,23 +21,24 @@ markupsafe==1.0 mistune==0.8.3 nbconvert==5.3.1 nbformat==4.4.0 -notebook==5.5.0 +notebook==5.6.0 pandocfilters==1.4.2 -parso==0.2.1 +parso==0.3.1 pexpect==4.6.0 pickleshare==0.7.4 +prometheus-client==0.3.0 prompt-toolkit==1.0.15 -ptyprocess==0.5.2 +ptyprocess==0.6.0 pygments==2.2.0 python-dateutil==2.7.3 -pyzmq==17.0.0 +pyzmq==17.1.0 qtconsole==4.3.1 send2trash==1.5.0 simplegeneric==0.8.1 six==1.11.0 terminado==0.8.1 testpath==0.3.1 -tornado==5.0.2 +tornado==5.1 traitlets==4.3.2 wcwidth==0.1.7 webencodings==0.5.1 diff --git a/requirements-sqlalchemy.txt b/requirements-sqlalchemy.txt index 8d49e86..f80fd17 100644 --- a/requirements-sqlalchemy.txt +++ b/requirements-sqlalchemy.txt @@ -5,23 +5,22 @@ bonobo-sqlalchemy==0.6.0 certifi==2018.4.16 chardet==3.0.4 colorama==0.3.9 -fs==2.0.23 -graphviz==0.8.3 -idna==2.6 +fs==2.0.25 +graphviz==0.8.4 +idna==2.7 jinja2==2.10 markupsafe==1.0 mondrian==0.7.0 packaging==17.1 -pbr==4.0.4 +pbr==4.1.1 psutil==5.4.6 pyparsing==2.2.0 python-slugify==1.2.5 -pytz==2018.4 -requests==2.18.4 +pytz==2018.5 +requests==2.19.1 six==1.11.0 -sqlalchemy==1.2.8 -stevedore==1.28.0 -typing==3.6.4 +sqlalchemy==1.2.10 +stevedore==1.29.0 unidecode==1.0.22 -urllib3==1.22 +urllib3==1.23 whistle==1.0.1 diff --git a/requirements.txt b/requirements.txt index d4696d1..faed2d4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,22 +3,21 @@ appdirs==1.4.3 certifi==2018.4.16 chardet==3.0.4 colorama==0.3.9 -fs==2.0.23 -graphviz==0.8.3 -idna==2.6 +fs==2.0.25 +graphviz==0.8.4 +idna==2.7 jinja2==2.10 markupsafe==1.0 mondrian==0.7.0 packaging==17.1 -pbr==4.0.4 +pbr==4.1.1 psutil==5.4.6 pyparsing==2.2.0 python-slugify==1.2.5 -pytz==2018.4 -requests==2.18.4 +pytz==2018.5 +requests==2.19.1 six==1.11.0 -stevedore==1.28.0 -typing==3.6.4 +stevedore==1.29.0 unidecode==1.0.22 -urllib3==1.22 +urllib3==1.23 whistle==1.0.1 diff --git a/setup.py b/setup.py index f9b7806..9e0aa4b 100644 --- a/setup.py +++ b/setup.py @@ -1,4 +1,4 @@ -# Generated by Medikit 0.6.3 on 2018-06-11. +# Generated by Medikit 0.6.3 on 2018-07-22. # All changes will be overriden. # Edit Projectfile and run “make update” (or “medikit update”) to regenerate. diff --git a/tests/execution/contexts/test_execution_contexts_graph.py b/tests/execution/contexts/test_execution_contexts_graph.py index 95b1ea4..e297c31 100644 --- a/tests/execution/contexts/test_execution_contexts_graph.py +++ b/tests/execution/contexts/test_execution_contexts_graph.py @@ -1,4 +1,5 @@ from bonobo import Graph +from bonobo.constants import EMPTY, BEGIN, END from bonobo.execution.contexts import GraphExecutionContext @@ -49,9 +50,8 @@ def test_lifecycle_of_graph_with_recoverable_error(): def test_lifecycle_of_graph_with_unrecoverable_error(): graph = Graph([1, 2, 3], raise_an_unrecoverrable_error, print) with GraphExecutionContext(graph) as context: - assert context.started - assert context.alive - assert not context.stopped + assert context.started and context.alive and not context.stopped + context.write(BEGIN, EMPTY, END) context.loop() assert context.started assert not context.alive diff --git a/tests/execution/contexts/test_node.py b/tests/execution/contexts/test_execution_contexts_node.py similarity index 100% rename from tests/execution/contexts/test_node.py rename to tests/execution/contexts/test_execution_contexts_node.py From 4e2cb29fc2a5137123a6f9e4be0d870f083aa068 Mon Sep 17 00:00:00 2001 From: Romain Dorgueil Date: Sun, 22 Jul 2018 07:54:22 +0200 Subject: [PATCH 3/3] Formatting. --- bonobo/config/configurables.py | 1 + bonobo/execution/contexts/node.py | 14 +++++++++++--- bonobo/registry.py | 2 ++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/bonobo/config/configurables.py b/bonobo/config/configurables.py index 98e21e7..81defaa 100644 --- a/bonobo/config/configurables.py +++ b/bonobo/config/configurables.py @@ -77,6 +77,7 @@ except ImportError: PartiallyConfigured = functools.partial else: + class PartiallyConfigured(_functools.partial): @property # TODO XXX cache this def _options_values(self): diff --git a/bonobo/execution/contexts/node.py b/bonobo/execution/contexts/node.py index 0affd29..c8d7733 100644 --- a/bonobo/execution/contexts/node.py +++ b/bonobo/execution/contexts/node.py @@ -30,6 +30,7 @@ class NodeExecutionContext(BaseContext, WithStatistics): a service implementation, or a value holder). """ + def __init__(self, wrapped, *, parent=None, services=None, _input=None, _outputs=None): """ Node execution context has the responsibility fo storing the state of a transformation during its execution. @@ -92,11 +93,15 @@ class NodeExecutionContext(BaseContext, WithStatistics): # Not normal to have a partially configured object here, so let's warn the user instead of having get into # the hard trouble of understanding that by himself. raise TypeError( - 'Configurables should be instanciated before execution starts.\nGot {!r}.\n'.format(self.wrapped) + 'Configurables should be instanciated before execution starts.\nGot {!r}.\n'.format( + self.wrapped + ) ) from exc else: raise TypeError( - 'Configurables should be instanciated before execution starts.\nGot {!r}.\n'.format(self.wrapped) + 'Configurables should be instanciated before execution starts.\nGot {!r}.\n'.format( + self.wrapped + ) ) self._stack.setup(self) except Exception: @@ -128,7 +133,10 @@ class NodeExecutionContext(BaseContext, WithStatistics): self._step() except InactiveReadableError: raise - except (NotImplementedError, UnrecoverableError, ): + except ( + NotImplementedError, + UnrecoverableError, + ): self.fatal(sys.exc_info()) # exit loop except Exception: # pylint: disable=broad-except self.error(sys.exc_info()) # does not exit loop diff --git a/bonobo/registry.py b/bonobo/registry.py index fba0c53..f45da4f 100644 --- a/bonobo/registry.py +++ b/bonobo/registry.py @@ -89,6 +89,7 @@ class Registry: default_registry = Registry() + def create_reader(name, *args, format=None, registry=default_registry, **kwargs): """ Create a reader instance, guessing its factory using filename (and eventually format). @@ -103,6 +104,7 @@ def create_reader(name, *args, format=None, registry=default_registry, **kwargs) """ return registry.get_reader_factory_for(name, format=format)(name, *args, **kwargs) + def create_writer(name, *args, format=None, registry=default_registry, **kwargs): """ Create a writer instance, guessing its factory using filename (and eventually format).