Skip to content

fix: prevent UnboundLocalError and TaskLock race condition in run_dbt_commands - #1404

Open
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/dbt-tasklock-race
Open

fix: prevent UnboundLocalError and TaskLock race condition in run_dbt_commands#1404
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/dbt-tasklock-race

Conversation

@sentry

@sentry sentry Bot commented Jun 9, 2026

Copy link
Copy Markdown

This PR addresses two related issues in the run_dbt_commands Celery task:

  1. UnboundLocalError: local variable 'taskprogress' referenced before assignment

    • Problem: An UnboundLocalError occurred if an exception was raised early in the try block (e.g., during TaskLock.objects.get_or_create) before the taskprogress variable was initialized. The except handler then tried to call taskprogress.add(), leading to the error.
    • Fix: taskprogress is now initialized to None before the try block, and its usage in the except handler is guarded with an if taskprogress is not None: check.
  2. Race condition in TaskLock.objects.get_or_create

    • Problem: The TaskLock model has a OneToOneField on orgtask, meaning orgtask_id is unique. However, get_or_create was attempting to look up by (orgtask, locked_by, celery_task_id). If two concurrent tasks for the same orgtask (but different celery_task_ids) ran, the get part would fail (due to celery_task_id mismatch), the create part would then fail with an IntegrityError (due to orgtask_id uniqueness), and the subsequent get retry would also fail, propagating the IntegrityError.
    • Fix: Modified TaskLock.objects.get_or_create to use only orgtask=orgtask for the lookup, moving locked_by and celery_task_id into the defaults dictionary. This ensures the lookup correctly identifies an existing lock by its unique orgtask. If a lock already exists, the celery_task_id and locked_by fields are explicitly updated to reflect the current task taking ownership.

Fixes DALGO-BACKEND-1EQ

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.86%. Comparing base (c6de97f) to head (2a3b674).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1404      +/-   ##
==========================================
+ Coverage   59.58%   59.86%   +0.28%     
==========================================
  Files         138      138              
  Lines       16544    16560      +16     
==========================================
+ Hits         9857     9913      +56     
+ Misses       6687     6647      -40     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants