fix: Avoid double transactions when cloning segments #6399
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Docker builds report
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| self.save_without_historical_record() | ||
|
|
||
| @transaction.atomic | ||
| def clone(self, is_revision: bool = False, **extra_attrs: typing.Any) -> "Segment": |
There was a problem hiding this comment.
Bug: Clone endpoint fails assertion without transaction wrapper
Removing @transaction.atomic from the clone method breaks the clone endpoint in api/segments/views.py. The clone method calls copy_rules_and_conditions_from, which asserts it must run inside a transaction (assert transaction.get_connection().in_atomic_block). While calls from the serializer and workflow service are wrapped in transactions, the view's clone action at line 140 calls source_segment.clone() directly without any transaction wrapper, causing this call path to fail with an assertion error.
There was a problem hiding this comment.
Good bot, but also bad bot — look out for outer @transaction.atomic usage in the serializers/services that call this method.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6399 +/- ##
==========================================
- Coverage 98.06% 98.06% -0.01%
==========================================
Files 1289 1289
Lines 46293 46292 -1
==========================================
- Hits 45395 45394 -1
Misses 898 898 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
This change helps us avoid transactions unnecessarily wrapped in transactions that might be the cause of lock contention in prod SaaS.
How did you test this code?
We'll see what CI says, and then test in production.