From b1710e85091845be20c9dffb46eb33d623622a90 Mon Sep 17 00:00:00 2001 From: Alistair Lynn Date: Wed, 25 Apr 2018 15:27:28 +0100 Subject: [PATCH 1/9] Add `--disallow-any-generics` to mypy --- routemaster/config/model.py | 2 +- setup.cfg | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/routemaster/config/model.py b/routemaster/config/model.py index d1a70a60..37b32cbf 100644 --- a/routemaster/config/model.py +++ b/routemaster/config/model.py @@ -160,7 +160,7 @@ class FeedConfig(NamedTuple): class Webhook(NamedTuple): """Configuration for webdook requests.""" - match: Pattern + match: Pattern[str] headers: Dict[str, str] diff --git a/setup.cfg b/setup.cfg index d2dbc0c4..1f42ccca 100644 --- a/setup.cfg +++ b/setup.cfg @@ -8,6 +8,7 @@ exclude=tests,migrations [mypy] ignore_missing_imports=true strict_optional=true +disallow_any_generics=true [coverage:run] branch=True From e72a2ef41ca1815625e821f1e9a4eafddd331cc6 Mon Sep 17 00:00:00 2001 From: Alistair Lynn Date: Wed, 25 Apr 2018 15:36:42 +0100 Subject: [PATCH 2/9] Fix most `--disallow-incomplete-defs` violations --- routemaster/cli.py | 2 +- routemaster/context.py | 2 +- routemaster/cron.py | 2 +- routemaster/feeds.py | 3 ++- routemaster/logging/base.py | 3 ++- routemaster/logging/plugins.py | 3 ++- routemaster/middleware.py | 2 +- routemaster/state_machine/actions.py | 5 ++++- routemaster/state_machine/api.py | 4 ++-- routemaster/validation.py | 4 ++-- 10 files changed, 18 insertions(+), 12 deletions(-) diff --git a/routemaster/cli.py b/routemaster/cli.py index 5446d87c..c1c61928 100644 --- a/routemaster/cli.py +++ b/routemaster/cli.py @@ -97,7 +97,7 @@ def serve(ctx, bind, debug, workers): # pragma: no cover cron_thread.stop() -def _validate_config(app: App): +def _validate_config(app: App) -> None: try: validate_config(app, app.config) except ValidationError as e: diff --git a/routemaster/context.py b/routemaster/context.py index 602c530b..38b167f4 100644 --- a/routemaster/context.py +++ b/routemaster/context.py @@ -81,7 +81,7 @@ def _pre_warm_feeds( label: str, accessed_variables: Iterable[str], logging_context, - ): + ) -> None: for accessed_variable in accessed_variables: parts = accessed_variable.split('.') diff --git a/routemaster/cron.py b/routemaster/cron.py index 855fc3f3..25b409f4 100644 --- a/routemaster/cron.py +++ b/routemaster/cron.py @@ -55,7 +55,7 @@ def process_job( # Bound when scheduling a specific job for a state fn: LabelStateProcessor, label_provider: LabelProvider, -): +) -> None: """Process a single instance of a single cron job.""" def _iter_labels_until_terminating(state_machine, state): diff --git a/routemaster/feeds.py b/routemaster/feeds.py index d8dc2bf1..bbe14053 100644 --- a/routemaster/feeds.py +++ b/routemaster/feeds.py @@ -5,10 +5,11 @@ import requests from dataclasses import InitVar, dataclass +from routemaster.config import StateMachine from routemaster.utils import get_path, template_url -def feeds_for_state_machine(state_machine) -> Dict[str, 'Feed']: +def feeds_for_state_machine(state_machine: StateMachine) -> Dict[str, 'Feed']: """Get a mapping of feed prefixes to unfetched feeds.""" return { x.name: Feed(x.url, state_machine.name) # type: ignore diff --git a/routemaster/logging/base.py b/routemaster/logging/base.py index 38b18b90..9e8c5b83 100644 --- a/routemaster/logging/base.py +++ b/routemaster/logging/base.py @@ -1,12 +1,13 @@ """Base class for logging plugins.""" import contextlib +from typing import Any class BaseLogger: """Base class for logging plugins.""" - def __init__(self, config, *args, **kwargs) -> None: + def __init__(self, config: Any, *args, **kwargs) -> None: self.config = config def init_flask(self, flask_app): diff --git a/routemaster/logging/plugins.py b/routemaster/logging/plugins.py index 76afc62e..20aa68a5 100644 --- a/routemaster/logging/plugins.py +++ b/routemaster/logging/plugins.py @@ -1,5 +1,6 @@ """Plugin loading and configuration.""" import importlib +from typing import List from routemaster.config import Config, LoggingPluginConfig from routemaster.logging.base import BaseLogger @@ -9,7 +10,7 @@ class PluginConfigurationException(Exception): """Raised to signal an invalid plugin that was loaded.""" -def register_loggers(config: Config): +def register_loggers(config: Config) -> List[BaseLogger]: """ Iterate through all plugins in the config file and instatiate them. """ diff --git a/routemaster/middleware.py b/routemaster/middleware.py index 66f3e3c2..62c63d25 100644 --- a/routemaster/middleware.py +++ b/routemaster/middleware.py @@ -11,7 +11,7 @@ ACTIVE_MIDDLEWARES = [] -def middleware(fn: WSGIMiddleware): +def middleware(fn: WSGIMiddleware) -> WSGIMiddleware: """Decorator: add `fn` to ACTIVE_MIDDLEWARES.""" ACTIVE_MIDDLEWARES.append(fn) return fn diff --git a/routemaster/state_machine/actions.py b/routemaster/state_machine/actions.py index 3733feb1..eb0ec42a 100644 --- a/routemaster/state_machine/actions.py +++ b/routemaster/state_machine/actions.py @@ -91,7 +91,10 @@ def process_action( return True -def _calculate_idempotency_token(label: LabelRef, latest_history) -> str: +def _calculate_idempotency_token( + label: LabelRef, + latest_history: History, +) -> str: """ We want to make sure that an action is only performed once. diff --git a/routemaster/state_machine/api.py b/routemaster/state_machine/api.py index c48a9db7..84b17dfc 100644 --- a/routemaster/state_machine/api.py +++ b/routemaster/state_machine/api.py @@ -151,7 +151,7 @@ def _process_transitions_for_metadata_update( label: LabelRef, state_machine: StateMachine, state_pending_update: State, -): +) -> None: with app.session.begin_nested(): lock_label(app, label) current_state = get_current_state(app, label, state_machine) @@ -219,7 +219,7 @@ def process_cron( app: App, state_machine: StateMachine, state: State, -): +) -> None: """ Cron event entrypoint. """ diff --git a/routemaster/validation.py b/routemaster/validation.py index 379be7ad..2fd92958 100644 --- a/routemaster/validation.py +++ b/routemaster/validation.py @@ -14,13 +14,13 @@ class ValidationError(Exception): pass -def validate_config(app: App, config: Config): +def validate_config(app: App, config: Config) -> None: """Validate that a given config satisfies invariants.""" for state_machine in config.state_machines.values(): _validate_state_machine(app, state_machine) -def _validate_state_machine(app: App, state_machine: StateMachine): +def _validate_state_machine(app: App, state_machine: StateMachine) -> None: """Validate that a given state machine is internally consistent.""" with app.new_session(): _validate_route_start_to_end(state_machine) From 550855fc5dc97a65678ce6efa74784f175b36725 Mon Sep 17 00:00:00 2001 From: Alistair Lynn Date: Wed, 25 Apr 2018 15:41:22 +0100 Subject: [PATCH 3/9] Disallow incomplete types --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 1f42ccca..66c43223 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,6 +9,7 @@ exclude=tests,migrations ignore_missing_imports=true strict_optional=true disallow_any_generics=true +disallow_incomplete_defs=true [coverage:run] branch=True From f5570bcfd00bb1e6b250aabc9d88447ed9ad40ae Mon Sep 17 00:00:00 2001 From: Alistair Lynn Date: Wed, 25 Apr 2018 15:44:04 +0100 Subject: [PATCH 4/9] isort --- routemaster/feeds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routemaster/feeds.py b/routemaster/feeds.py index bbe14053..de1cfb7f 100644 --- a/routemaster/feeds.py +++ b/routemaster/feeds.py @@ -5,8 +5,8 @@ import requests from dataclasses import InitVar, dataclass -from routemaster.config import StateMachine from routemaster.utils import get_path, template_url +from routemaster.config import StateMachine def feeds_for_state_machine(state_machine: StateMachine) -> Dict[str, 'Feed']: From e87e58067fd42bb766836d73637818220411848f Mon Sep 17 00:00:00 2001 From: Dan Palmer Date: Sat, 19 May 2018 10:28:13 +0100 Subject: [PATCH 5/9] Unused --- routemaster/tests/test_layering.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/routemaster/tests/test_layering.py b/routemaster/tests/test_layering.py index 01d10e70..696174ac 100644 --- a/routemaster/tests/test_layering.py +++ b/routemaster/tests/test_layering.py @@ -136,8 +136,6 @@ def test_layers(): last_import_source = None - top_level_import = False - for instruction in dis.get_instructions(code): if instruction.opname == 'IMPORT_NAME': import_target = instruction.argval From 4190151a219495f1d6517608d604eff9d92bae93 Mon Sep 17 00:00:00 2001 From: Dan Palmer Date: Sat, 19 May 2018 10:28:31 +0100 Subject: [PATCH 6/9] Fix layering for feeds --- routemaster/tests/test_layering.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routemaster/tests/test_layering.py b/routemaster/tests/test_layering.py index 696174ac..0b597c2e 100644 --- a/routemaster/tests/test_layering.py +++ b/routemaster/tests/test_layering.py @@ -27,7 +27,6 @@ ('exit_conditions', 'utils'), ('context', 'utils'), - ('context', 'feeds'), ('config', 'exit_conditions'), ('config', 'context'), @@ -57,6 +56,7 @@ ('state_machine', 'webhooks'), ('feeds', 'utils'), + ('feeds', 'config'), ('webhooks', 'config'), From 14ecbb36f4dc71f5904fc48d5dfa754fdb069a55 Mon Sep 17 00:00:00 2001 From: Dan Palmer Date: Sat, 19 May 2018 10:28:43 +0100 Subject: [PATCH 7/9] Break circular import --- routemaster/context.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/routemaster/context.py b/routemaster/context.py index 38b167f4..65d629e0 100644 --- a/routemaster/context.py +++ b/routemaster/context.py @@ -1,10 +1,12 @@ """Context definition for exit condition programs.""" import datetime -from typing import Any, Dict, Iterable, Optional, Sequence +from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Sequence -from routemaster.feeds import Feed from routemaster.utils import get_path +if TYPE_CHECKING: + from routemaster.feeds import Feed # noqa + class Context(object): """Execution context for exit condition programs.""" @@ -15,7 +17,7 @@ def __init__( label: str, metadata: Dict[str, Any], now: datetime.datetime, - feeds: Dict[str, Feed], + feeds: Dict[str, 'Feed'], accessed_variables: Iterable[str], current_history_entry: Optional[Any], feed_logging_context, From 5f5d1fe3820c5f0f4550fbc4b3203e1a7f66da64 Mon Sep 17 00:00:00 2001 From: Dan Palmer Date: Sat, 19 May 2018 10:50:46 +0100 Subject: [PATCH 8/9] Ignore if TYPE_CHECKING for layering violations --- routemaster/tests/test_layering.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/routemaster/tests/test_layering.py b/routemaster/tests/test_layering.py index 0b597c2e..c99b0e51 100644 --- a/routemaster/tests/test_layering.py +++ b/routemaster/tests/test_layering.py @@ -135,8 +135,34 @@ def test_layers(): code = compile(contents, module_name, 'exec') last_import_source = None + last_load_name = None + skip_to_offset = None + + for offset, instruction in enumerate(dis.get_instructions(code)): + + # Skip over checking code that is within a block like this: + # if TYPE_CHECKING: + # import... + if skip_to_offset == offset: + skip_to_offset = None + elif skip_to_offset is not None: + continue + + if instruction.opname == 'LOAD_NAME': + last_load_name = instruction.argval + continue + + elif ( + instruction.opname == 'POP_JUMP_IF_FALSE' and + last_load_name == 'TYPE_CHECKING' + ): + skip_to_offset = instruction.argval + continue + + last_load_name = None + + # Now do the actual import checking - for instruction in dis.get_instructions(code): if instruction.opname == 'IMPORT_NAME': import_target = instruction.argval From 49c12efd022af81e864cd4a8293e33d10ad6a0bc Mon Sep 17 00:00:00 2001 From: Dan Palmer Date: Sat, 19 May 2018 11:45:52 +0100 Subject: [PATCH 9/9] Fix up types with new mypy strictness --- routemaster/context.py | 23 +++++++++++++++++++---- routemaster/feeds.py | 4 +++- routemaster/logging/base.py | 2 +- routemaster/logging/python_logger.py | 8 ++++++-- routemaster/logging/split_logger.py | 9 ++++++--- routemaster/logging/tests/test_loggers.py | 2 +- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/routemaster/context.py b/routemaster/context.py index 65d629e0..ef1a141a 100644 --- a/routemaster/context.py +++ b/routemaster/context.py @@ -1,11 +1,26 @@ """Context definition for exit condition programs.""" import datetime -from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Sequence +from typing import ( + TYPE_CHECKING, + Any, + Dict, + Callable, + Iterable, + Optional, + Sequence, + ContextManager, +) from routemaster.utils import get_path if TYPE_CHECKING: - from routemaster.feeds import Feed # noqa + import requests # noqa + from routemaster.feeds import Feed, FeedResponseLogger # noqa + +FeedLoggingContext = Callable[ + [str], + ContextManager[Callable[['requests.Response'], None]], +] class Context(object): @@ -20,7 +35,7 @@ def __init__( feeds: Dict[str, 'Feed'], accessed_variables: Iterable[str], current_history_entry: Optional[Any], - feed_logging_context, + feed_logging_context: FeedLoggingContext, ) -> None: """Create an execution context.""" if now.tzinfo is None: @@ -82,7 +97,7 @@ def _pre_warm_feeds( self, label: str, accessed_variables: Iterable[str], - logging_context, + logging_context: FeedLoggingContext, ) -> None: for accessed_variable in accessed_variables: parts = accessed_variable.split('.') diff --git a/routemaster/feeds.py b/routemaster/feeds.py index de1cfb7f..797716de 100644 --- a/routemaster/feeds.py +++ b/routemaster/feeds.py @@ -8,6 +8,8 @@ from routemaster.utils import get_path, template_url from routemaster.config import StateMachine +FeedResponseLogger = Callable[[requests.Response], None] + def feeds_for_state_machine(state_machine: StateMachine) -> Dict[str, 'Feed']: """Get a mapping of feed prefixes to unfetched feeds.""" @@ -43,7 +45,7 @@ class Feed: def prefetch( self, label: str, - log_response: Callable[[requests.Response], None] = lambda x: None, + log_response: FeedResponseLogger = lambda x: None, ) -> None: """Trigger the fetching of a feed's data.""" if self.data is not None: diff --git a/routemaster/logging/base.py b/routemaster/logging/base.py index 4d8dd70e..c5b82f56 100644 --- a/routemaster/logging/base.py +++ b/routemaster/logging/base.py @@ -7,7 +7,7 @@ class BaseLogger: """Base class for logging plugins.""" - def __init__(self, config: Any, *args, **kwargs) -> None: + def __init__(self, config: Any, **kwargs: str) -> None: self.config = config def init_flask(self, flask_app): diff --git a/routemaster/logging/python_logger.py b/routemaster/logging/python_logger.py index ad658385..abdb3f75 100644 --- a/routemaster/logging/python_logger.py +++ b/routemaster/logging/python_logger.py @@ -3,15 +3,19 @@ import time import logging import contextlib +from typing import TYPE_CHECKING from routemaster.logging.base import BaseLogger +if TYPE_CHECKING: + from routemaster.config import Config # noqa + class PythonLogger(BaseLogger): """Routemaster logging interface for Python's logging library.""" - def __init__(self, *args, log_level: str) -> None: - super().__init__(*args) + def __init__(self, config: 'Config', log_level: str) -> None: + super().__init__(config) logging.basicConfig( format=( diff --git a/routemaster/logging/split_logger.py b/routemaster/logging/split_logger.py index 8021619c..9c83f694 100644 --- a/routemaster/logging/split_logger.py +++ b/routemaster/logging/split_logger.py @@ -2,16 +2,19 @@ import functools import contextlib -from typing import List +from typing import TYPE_CHECKING, List from routemaster.logging.base import BaseLogger +if TYPE_CHECKING: + from routemaster.config import Config # noqa + class SplitLogger(BaseLogger): """Proxies logging calls to all loggers in a list.""" - def __init__(self, *args, loggers: List[BaseLogger]) -> None: - super().__init__(*args) + def __init__(self, config: 'Config', loggers: List[BaseLogger]) -> None: + super().__init__(config) self.loggers = loggers diff --git a/routemaster/logging/tests/test_loggers.py b/routemaster/logging/tests/test_loggers.py index 7933c71c..c5adc0dc 100644 --- a/routemaster/logging/tests/test_loggers.py +++ b/routemaster/logging/tests/test_loggers.py @@ -18,7 +18,7 @@ ]}), (SplitLogger, {'loggers': [ PythonLogger(None, log_level='WARN'), - BaseLogger(None, {}), + BaseLogger(None), ]}), ]