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

Fix transaction issues #36

Merged
merged 7 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[run]
branch = True

[report]
ignore_errors = True
include = side_effects/*
6 changes: 1 addition & 5 deletions .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,10 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python: ["3.8", "3.9", "3.10", "3.11", "3.12"]
python: ["3.9", "3.10", "3.11", "3.12"]
# build LTS version, next version, HEAD
django: ["32", "42", "50", "main"]
exclude:
- python: "3.8"
django: "50"
- python: "3.8"
django: "main"
- python: "3.9"
django: "50"
- python: "3.9"
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.coverage
.tox
.venv
.vscode
*.egg-info
*.lock
*.pyc
Expand Down
11 changes: 3 additions & 8 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
repos:
# python code formatting - will amend files
- repo: https://github.com/ambv/black
rev: 23.10.1
hooks:
- id: black

- repo: https://github.com/charliermarsh/ruff-pre-commit
# Ruff version.
rev: "v0.1.5"
rev: "v0.2.1"
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
- id: ruff-format

# python static type checking
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.7.0
rev: v1.8.0
hooks:
- id: mypy
args:
Expand Down
10 changes: 5 additions & 5 deletions .ruff.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
line-length = 88
ignore = [
lint.ignore = [
"D100", # Missing docstring in public module
"D101", # Missing docstring in public class
"D102", # Missing docstring in public method
Expand All @@ -22,7 +22,7 @@ ignore = [
"D417",
"D417", # Missing argument description in the docstring
]
select = [
lint.select = [
"A", # flake8 builtins
"C9", # mcabe
"D", # pydocstyle
Expand All @@ -34,13 +34,13 @@ select = [
"W", # pycodestype (warnings)
]

[isort]
[lint.isort]
combine-as-imports = true

[mccabe]
[lint.mccabe]
max-complexity = 8

[per-file-ignores]
[lint.per-file-ignores]
"*tests/*" = [
"D205", # 1 blank line required between summary line and description
"D400", # First line should end with a period
Expand Down
4 changes: 1 addition & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ classifiers = [
"Framework :: Django :: 5.0",
"Operating System :: OS Independent",
"Programming Language :: Python :: 3 :: Only",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
Expand All @@ -26,11 +25,10 @@ classifiers = [
packages = [{ include = "side_effects" }]

[tool.poetry.dependencies]
python = "^3.8"
python = "^3.9"
django = "^3.2 || ^4.0 || ^5.0 "

[tool.poetry.dev-dependencies]
black = "*"
coverage = "*"
mypy = "*"
pre-commit = "*"
Expand Down
10 changes: 1 addition & 9 deletions side_effects/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from django.http import HttpResponse

from . import registry
from .settings import TEST_MODE


def http_response_check(response: HttpResponse) -> bool:
Expand Down Expand Up @@ -63,14 +62,7 @@ def inner_func(*args: Any, **kwargs: Any) -> Any:
# if the exit test fails we go no further
if not run_on_exit(return_value):
return return_value
# when in TEST_MODE do not defer the side-effects, as they
# are never fired anyway.
registry_func = (
registry.run_side_effects
if TEST_MODE
else registry.run_side_effects_on_commit
)
registry_func(label, *args, return_value=return_value, **kwargs)
registry.run_side_effects(label, *args, return_value=return_value, **kwargs)
Comment on lines -66 to +65
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Part one of the change is to always call run_side_effects rather than adding in fancy logic to the decorator. Simpler.

return return_value

return inner_func
Expand Down
82 changes: 17 additions & 65 deletions side_effects/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ def docstring(func: Callable) -> list[str] | None:
return None


class SideEffectsTestFailure(Exception):
def __init__(self, label: str):
super().__init__(f"Side-effects for '{label}' aborted; TEST_MODE_FAIL=True")


class SignatureMismatch(Exception):
def __init__(self, func: Callable):
super().__init__(
Expand Down Expand Up @@ -116,52 +111,12 @@ def disable(self) -> None:
def enable(self) -> None:
self._suppress = False

def _run_side_effects(
self, label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
) -> None:
if settings.TEST_MODE_FAIL:
raise SideEffectsTestFailure(label)
for func in self[label]:
_run_func(func, *args, return_value=return_value, **kwargs)

def run_side_effects(
self, label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
) -> None:
"""
Run registered side-effects functions, or suppress as appropriate.

If TEST_MODE is on, or the _suppress attr is True, then the side-effects
are not run, but the `suppressed_side_effect` signal is sent - this is
primarily used by the disable_side_effects context manager to register
which side-effects events were suppressed (for testing purposes).

NB even if the side-effects themselves are not run, this method will try
to bind all of the receiver functions - this is to ensure that incompatible
functions fail hard and early.

"""
# TODO: this is all becoming over-complex - need to simplify this
self.try_bind_all(label, *args, return_value=return_value, **kwargs)
if self.is_suppressed:
self.suppressed_side_effect.send(Registry, label=label)
else:
self._run_side_effects(label, *args, return_value=return_value, **kwargs)
Comment on lines -143 to -148
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODONE - it was too complex so I have removed it.


def try_bind_all(
self, label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
) -> None:
"""
Test all receivers for signature compatibility.

Raise SignatureMismatch if any function does not match.

"""
"""Run all registered side-effects functions."""
for func in self[label]:
if not (
try_bind(func, *args, return_value=return_value, **kwargs)
or try_bind(func, *args, **kwargs)
):
raise SignatureMismatch(func)
_run_func(func, *args, return_value=return_value, **kwargs)


class disable_side_effects:
Expand All @@ -177,8 +132,7 @@ class disable_side_effects:
"""

def __init__(self) -> None:
self.events = [] # type: List[str]
pass
self.events: list[str] = []

def __enter__(self) -> list[str]:
_registry.suppressed_side_effect.connect(self.on_event, dispatch_uid="suppress")
Expand All @@ -204,22 +158,20 @@ def run_side_effects(
label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
) -> None:
"""Run all of the side-effect functions registered for a label."""
_registry.run_side_effects(label, *args, return_value=return_value, **kwargs)


def run_side_effects_on_commit(
label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
) -> None:
"""Run all of the side-effects after current transaction on_commit."""
transaction.on_commit(
partial(
_registry.run_side_effects,
label,
*args,
return_value=return_value,
**kwargs,
# if the registry is suppressed we are inside disable_side_effects,
# so we send the signal and return early.
if _registry.is_suppressed:
_registry.suppressed_side_effect.send(Registry, label=label)
else:
transaction.on_commit(
partial(
_registry.run_side_effects,
label,
*args,
return_value=return_value,
**kwargs,
)
)
)


def _run_func(
Expand All @@ -239,7 +191,7 @@ def _run_func(
raise
except Exception: # noqa: B902
logger.exception("Error running side_effect function '%s'", fname(func))
if settings.ABORT_ON_ERROR or settings.TEST_MODE_FAIL:
if settings.ABORT_ON_ERROR:
raise


Expand Down
6 changes: 0 additions & 6 deletions side_effects/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,3 @@ def get_setting(setting_name: str, default_value: Any) -> Any:
# In test mode no side-effects are run
# Default = False
TEST_MODE: bool = get_setting("SIDE_EFFECTS_TEST_MODE", False)

# In FAIL test mode any call to run_side_effects will raise an Exception
# This is used to uncover any tests that are running side-effects when
# they shouldn't be.
# Default = False
TEST_MODE_FAIL: bool = get_setting("SIDE_EFFECTS_TEST_MODE_FAIL", False)
7 changes: 7 additions & 0 deletions side_effects/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from django.dispatch import Signal

# if using the disable_side_effects context manager or decorator,
# then this signal is used to communicate details of events that
# would have fired, but have been suppressed.
# RemovedInDjango40Warning: providing_args=["label"]
suppressed_side_effect = Signal()
10 changes: 6 additions & 4 deletions tests/test_checks.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
from typing import Any

from django.test import TestCase

from side_effects import checks, registry


class SystemCheckTests(TestCase):
def test_multiple_functions(self):
def foo():
def test_multiple_functions(self) -> None:
def foo() -> None:
pass

def bar():
def bar() -> None:
pass

def baz(arg1):
def baz(arg1: Any) -> None:
pass

registry._registry.clear()
Expand Down
Loading
Loading