-
Notifications
You must be signed in to change notification settings - Fork 4
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
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
4047f5a
Fix issue with disable_side_effects
hugorodgerbrown 0bd8ae4
Add more tests
hugorodgerbrown 1a87110
Remove Python 3.8 (typing)
hugorodgerbrown 2203ade
Add additional tests
hugorodgerbrown d96ae1e
Replace Black pre-commit with ruff format
hugorodgerbrown 13330f0
Added tx tests
hugorodgerbrown 8f21d92
Remove TEST_FAIL_MODE
hugorodgerbrown File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
[run] | ||
branch = True | ||
|
||
[report] | ||
ignore_errors = True | ||
include = side_effects/* |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
.coverage | ||
.tox | ||
.venv | ||
.vscode | ||
*.egg-info | ||
*.lock | ||
*.pyc | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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__( | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
|
@@ -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") | ||
|
@@ -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( | ||
|
@@ -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 | ||
|
||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.