Skip to content

Commit

Permalink
[#37] Explicitly handle transactions in the runner
Browse files Browse the repository at this point in the history
  • Loading branch information
swrichards committed Dec 18, 2024
1 parent 388dc94 commit b2feb26
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from pathlib import Path

from django.core.management import BaseCommand, CommandError
from django.db import transaction

from django_setup_configuration.exceptions import ValidateRequirementsFailure
from django_setup_configuration.runner import SetupConfigurationRunner
Expand Down Expand Up @@ -33,7 +32,6 @@ def add_arguments(self, parser):
help="Path to YAML file containing the configurations",
)

@transaction.atomic
def handle(self, **options):
yaml_file = Path(options["yaml_file"]).resolve()
if not yaml_file.exists():
Expand Down
25 changes: 22 additions & 3 deletions django_setup_configuration/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Any, Generator

from django.conf import settings
from django.db import transaction
from django.utils.module_loading import import_string

from pydantic import ValidationError
Expand Down Expand Up @@ -157,7 +158,8 @@ def _execute_step(
step_exc = None

try:
step.execute(config_model)
with transaction.atomic():
step.execute(config_model)
except BaseException as exc:
step_exc = exc
finally:
Expand Down Expand Up @@ -207,8 +209,25 @@ def execute_all_iter(self) -> Generator[StepExecutionResult, Any, None]:
Generator[StepExecutionResult, Any, None]: The results of each step's
execution.
"""
for step in self.enabled_steps:
yield self._execute_step(step)

# Not the most elegant approach to rollbacks, but it's preferable to the
# pitfalls of manual transaction management. We want all steps to run and only
# rollback at the end, hence intra-step exceptions are caught and persisted.
class Rollback(BaseException):
pass

try:
with transaction.atomic():
results = []
for step in self.enabled_steps:
result = self._execute_step(step)
results.append(result)
yield result

if any(result.run_exception for result in results):
raise Rollback # Trigger the rollback
except Rollback:
pass

def execute_all(self) -> list[StepExecutionResult]:
"""
Expand Down
2 changes: 2 additions & 0 deletions tests/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from django_setup_configuration.test_utils import execute_single_step
from tests.conftest import TestStep, TestStepConfig

pytestmark = pytest.mark.django_db


def test_runner_raises_on_non_existent_step_module_path(test_step_yaml_path):
with pytest.raises(ConfigurationException):
Expand Down
2 changes: 2 additions & 0 deletions tests/test_test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from django_setup_configuration.test_utils import execute_single_step
from tests.conftest import TestStep

pytestmark = pytest.mark.django_db


def test_exception_during_execute_step_is_immediately_raised(
step_execute_mock,
Expand Down
104 changes: 104 additions & 0 deletions tests/test_transactions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
from unittest import mock

from django.contrib.auth.models import User

import pytest

from django_setup_configuration.configuration import BaseConfigurationStep
from django_setup_configuration.models import ConfigurationModel
from tests.conftest import TestStep

pytestmark = pytest.mark.django_db


side_effect_test_func = lambda: ...


class TransactionTestConfigurationModel(ConfigurationModel):

username: str


class TransactionTestConfigurationStep(
BaseConfigurationStep[TransactionTestConfigurationModel]
):

config_model = TransactionTestConfigurationModel
enable_setting = "transaction_test_configuration_enabled"
namespace = "transaction_test_configuration"
verbose_name = "Transaction Test Configuration"

def execute(self, model) -> None:
User.objects.create_user(
username=model.username,
password="secret",
)

side_effect_test_func()


@pytest.fixture()
def valid_config_object(test_step_valid_config):
return {
"transaction_test_configuration_enabled": True,
"transaction_test_configuration": {"username": "alice"},
} | test_step_valid_config


def test_runner_rolls_back_all_on_failing_step(
runner_factory, valid_config_object, step_execute_mock
):
runner = runner_factory(
steps=[TransactionTestConfigurationStep, TestStep],
object_source=valid_config_object,
)
exc = Exception()
step_execute_mock.side_effect = exc

user_configuration_step_result, test_step_result = runner.execute_all()

# Initial run is rolled back, so no objects created
assert test_step_result.has_run
assert test_step_result.run_exception is exc

assert user_configuration_step_result.has_run
assert user_configuration_step_result.run_exception is None
assert User.objects.count() == 0

# Subsequent run does not raise, so the objects are created
step_execute_mock.side_effect = None

user_configuration_step_result, test_step_result = runner.execute_all()

assert test_step_result.has_run
assert test_step_result.run_exception is None

assert user_configuration_step_result.has_run
assert user_configuration_step_result.run_exception is None
assert User.objects.count() == 1


def test_runner_rolls_back_on_executing_single_step(
runner_factory, valid_config_object
):
runner = runner_factory(
steps=[TransactionTestConfigurationStep, TestStep],
object_source=valid_config_object,
)
with mock.patch("tests.test_transactions.side_effect_test_func") as m:
exc = Exception()
m.side_effect = exc

user_configuration_step_result = runner._execute_step(
runner.configured_steps[0]
)

assert user_configuration_step_result.has_run
assert user_configuration_step_result.run_exception is exc
assert User.objects.count() == 0

user_configuration_step_result = runner._execute_step(runner.configured_steps[0])

assert user_configuration_step_result.has_run
assert user_configuration_step_result.run_exception is None
assert User.objects.count() == 1

0 comments on commit b2feb26

Please sign in to comment.