custom_check: Add rule to avoid creating savepoints.

This commit adds a custom rule to check python files
and raise lint error if they have transaction.atomic used
without any argument or savepoint=True is used.

It helps to avoid creating unintended savepoints in the future.
This commit is contained in:
Prakhar Pratyush
2024-11-21 23:44:06 +05:30
committed by Tim Abbott
parent 62b3e49075
commit c5b3d2e434
3 changed files with 19 additions and 4 deletions

View File

@@ -1816,7 +1816,7 @@ class BillingSession(ABC):
) )
# TODO: The correctness of this relies on user creation, deactivation, etc being # TODO: The correctness of this relies on user creation, deactivation, etc being
# in a transaction.atomic() with the relevant RealmAuditLog entries # in a transaction.atomic block with the relevant RealmAuditLog entries
with transaction.atomic(durable=True): with transaction.atomic(durable=True):
# We get the current license count here in case the number of billable # We get the current license count here in case the number of billable
# licenses has changed since the upgrade process began. # licenses has changed since the upgrade process began.

View File

@@ -462,8 +462,23 @@ python_rules = RuleList(
"description": 'A mock function is missing a leading "assert_"', "description": 'A mock function is missing a leading "assert_"',
}, },
{ {
"pattern": "@transaction.atomic\\(\\)", "pattern": r"transaction\.atomic$|transaction\.atomic\(\)|savepoint=True",
"description": "Use @transaction.atomic as function decorator for consistency.", "description": "Use 'durable=True' or 'savepoint=False' argument explicitly to avoid the possibility of creating savepoints.",
"exclude": {"confirmation/migrations/", "zerver/migrations/", "zilencer/migrations/"},
"exclude_line": {
(
"zerver/actions/create_user.py",
"with suppress(IntegrityError), transaction.atomic(savepoint=True):",
),
(
"zerver/lib/test_classes.py",
"with transaction.atomic(savepoint=True):",
),
(
"zerver/tests/test_subs.py",
"with transaction.atomic(savepoint=True), self.assertRaises(JsonableError):",
),
},
}, },
*whitespace_rules, *whitespace_rules,
*shebang_rules, *shebang_rules,

View File

@@ -132,7 +132,7 @@ def run_archiving(
# archived-and-deleted or not transactionally. # archived-and-deleted or not transactionally.
# #
# We implement this design by executing queries that archive messages and their related objects # We implement this design by executing queries that archive messages and their related objects
# (such as UserMessage, Reaction, and Attachment) inside the same transaction.atomic() block. # (such as UserMessage, Reaction, and Attachment) inside the same transaction.atomic block.
assert type in (ArchiveTransaction.MANUAL, ArchiveTransaction.RETENTION_POLICY_BASED) assert type in (ArchiveTransaction.MANUAL, ArchiveTransaction.RETENTION_POLICY_BASED)
if chunk_size is not None: if chunk_size is not None: