Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More strictness in mypy (WIP) #61

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion routemaster/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion routemaster/config/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class FeedConfig(NamedTuple):

class Webhook(NamedTuple):
"""Configuration for webdook requests."""
match: Pattern
match: Pattern[str]
headers: Dict[str, str]


Expand Down
29 changes: 23 additions & 6 deletions routemaster/context.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
"""Context definition for exit condition programs."""
import datetime
from typing import Any, Dict, Iterable, Optional, Sequence
from typing import (
TYPE_CHECKING,
Any,
Dict,
Callable,
Iterable,
Optional,
Sequence,
ContextManager,
)

from routemaster.feeds import Feed
from routemaster.utils import get_path

if TYPE_CHECKING:
import requests # noqa
from routemaster.feeds import Feed, FeedResponseLogger # noqa

FeedLoggingContext = Callable[
[str],
ContextManager[Callable[['requests.Response'], None]],
]


class Context(object):
"""Execution context for exit condition programs."""
Expand All @@ -15,10 +32,10 @@ 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,
feed_logging_context: FeedLoggingContext,
) -> None:
"""Create an execution context."""
if now.tzinfo is None:
Expand Down Expand Up @@ -80,8 +97,8 @@ 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('.')

Expand Down
2 changes: 1 addition & 1 deletion routemaster/cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
7 changes: 5 additions & 2 deletions routemaster/feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
from dataclasses import InitVar, dataclass

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) -> 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
Expand Down Expand Up @@ -42,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:
Expand Down
3 changes: 2 additions & 1 deletion routemaster/logging/base.py
Original file line number Diff line number Diff line change
@@ -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, **kwargs: str) -> None:
self.config = config

def init_flask(self, flask_app):
Expand Down
3 changes: 2 additions & 1 deletion routemaster/logging/plugins.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
"""
Expand Down
8 changes: 6 additions & 2 deletions routemaster/logging/python_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=(
Expand Down
9 changes: 6 additions & 3 deletions routemaster/logging/split_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion routemaster/logging/tests/test_loggers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
]}),
(SplitLogger, {'loggers': [
PythonLogger(None, log_level='WARN'),
BaseLogger(None, {}),
BaseLogger(None),
]}),
]

Expand Down
2 changes: 1 addition & 1 deletion routemaster/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion routemaster/state_machine/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,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.

Expand Down
4 changes: 2 additions & 2 deletions routemaster/state_machine/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -223,7 +223,7 @@ def process_cron(
app: App,
state_machine: StateMachine,
state: State,
):
) -> None:
"""
Cron event entrypoint.
"""
Expand Down
30 changes: 27 additions & 3 deletions routemaster/tests/test_layering.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
('exit_conditions', 'utils'),

('context', 'utils'),
('context', 'feeds'),

('config', 'exit_conditions'),
('config', 'context'),
Expand Down Expand Up @@ -57,6 +56,7 @@
('state_machine', 'webhooks'),

('feeds', 'utils'),
('feeds', 'config'),

('webhooks', 'config'),

Expand Down Expand Up @@ -135,10 +135,34 @@ def test_layers():
code = compile(contents, module_name, 'exec')

last_import_source = None
last_load_name = None
skip_to_offset = None

top_level_import = False
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

Expand Down
4 changes: 2 additions & 2 deletions routemaster/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ no-accept-encodings=true
[mypy]
ignore_missing_imports=true
strict_optional=true
disallow_any_generics=true
Copy link
Contributor

@mthpower mthpower Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an example of what this would disallow? (the documentation isn't very clear on this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foo: List rather than foo: List[int]

disallow_incomplete_defs=true

[coverage:run]
branch=True
Expand Down