-
Notifications
You must be signed in to change notification settings - Fork 5.3k
fix(cycle): ignore_conflicts on add-issue bulk_create to avoid 500 on concurrent adds #9605
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
base: preview
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -274,8 +274,20 @@ def create(self, request, slug, project_id, cycle_id): | |
| for issue in new_issues | ||
| ], | ||
| batch_size=10, | ||
| # Concurrent add-issue requests can both pass the unlocked | ||
| # existing-issue check above and insert the same (cycle, issue), | ||
| # violating the cycle_issue_when_deleted_at_null unique constraint. | ||
| # Skip the duplicates instead of raising, matching the module and | ||
| # external-API equivalents. | ||
| ignore_conflicts=True, | ||
| ) | ||
|
|
||
| # ON CONFLICT DO NOTHING does not populate PKs on skipped rows, so a | ||
| # concurrent request that lost the race gets its duplicates back here | ||
| # with pk=None. Drop them before the activity log below, otherwise it | ||
| # records a "created" entry for an insert that was silently discarded. | ||
| created_records = [record for record in created_records if record.pk is not None] | ||
|
|
||
|
Comment on lines
+285
to
+290
Contributor
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. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Primary-key definitions ---'
rg -n -C 10 \
'class (CycleIssue|ProjectBaseModel|BaseModel)|id\s*=\s*models\.(UUIDField|AutoField|BigAutoField)' \
apps/api/plane --glob '*.py'
printf '%s\n' '--- Declared Django version ---'
rg -n 'Django' apps/api \
--glob 'requirements*' \
--glob 'pyproject.toml' \
--glob 'setup.cfg' || true
printf '%s\n' '--- Regression tests ---'
rg -n -C 10 \
'CycleIssueViewSet|ignore_conflicts=True|cycle\.activity\.created|concurrent' \
apps/api \
--glob '*test*.py' \
--glob '*tests*.py' || trueRepository: makeplane/plane Length of output: 24515 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Target view and activity logging ---'
sed -n '220,315p' apps/api/plane/app/views/cycle/issue.py
printf '%s\n' '--- CycleIssue model and constraints ---'
sed -n '95,135p' apps/api/plane/db/models/cycle.py
printf '%s\n' '--- All CycleIssue bulk-create paths and activity payloads ---'
rg -n -C 8 \
'CycleIssue\.objects\.bulk_create|bulk_create\(.*ignore_conflicts|created_records|activity\.created' \
apps/api/plane --glob '*.py'
printf '%s\n' '--- Django dependency declarations ---'
rg -n -i '(^|[<=> ])django([<=> ].*)?$|django==' . \
--glob 'requirements*' --glob 'pyproject.toml' --glob 'setup.cfg' --glob 'Pipfile*' \
--glob '!node_modules/**' || trueRepository: makeplane/plane Length of output: 50371 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
import re
import uuid
model = Path("apps/api/plane/db/models/base.py").read_text()
cycle = Path("apps/api/plane/db/models/cycle.py").read_text()
view = Path("apps/api/plane/app/views/cycle/issue.py").read_text()
assert re.search(
r"id\s*=\s*models\.UUIDField\([^)]*default\s*=\s*uuid\.uuid4[^)]*primary_key\s*=\s*True",
model,
), "BaseModel does not declare a preassigned UUID primary key"
assert re.search(r"class\s+CycleIssue\s*\(\s*ProjectBaseModel\s*\)", cycle)
assert "serializers.serialize(\"json\", created_records)" in view
# Model the relevant Django behavior: UUIDField(default=uuid.uuid4)
# assigns the primary key while each object is constructed. An ignored
# insert does not clear that Python attribute.
successful = {"id": uuid.uuid4()}
skipped = {"id": uuid.uuid4()}
returned = [successful, skipped]
assert all(record["id"] is not None for record in returned)
# A database query by the attempted UUID distinguishes the inserted object
# from the conflict-skipped object, because the skipped UUID is not persisted.
persisted_ids = {successful["id"]}
inserted = [record for record in returned if record["id"] in persisted_ids]
assert inserted == [successful]
print("BaseModel uses a preassigned UUID primary key.")
print("CycleIssue inherits BaseModel through ProjectBaseModel.")
print("Both successful and conflict-skipped objects retain non-null pk values.")
print("A database-backed lookup of attempted UUIDs distinguishes inserted rows.")
print("created_records is serialized into cycle.activity.created.")
PYRepository: makeplane/plane Length of output: 460 Use a database-backed inserted-row check instead of the
🤖 Prompt for AI AgentsSource: MCP tools |
||
| # Updated Issues | ||
| updated_records = [] | ||
| update_cycle_issue_activity = [] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.