fix(modeling): fix generic delta errors and s3 fnf errors from empty queryable tables#60688
Merged
Conversation
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
posthog/temporal/data_modeling/activities/materialize_view.py:579-588
**Drain loop not cancellation-safe under activity cancellation**
If Temporal cancels the activity while the drain loop is sleeping, `await asyncio.sleep(0.05)` raises `CancelledError`, which propagates immediately out of the `finally` block — `await producer_task` is never reached. If the producer is simultaneously blocked on `await asyncio.to_thread(batch_queue.put, SENTINEL)` (queue still full because nothing drained it), the thread pool thread holding that `put` is orphaned until the GC eventually drops the queue, and the producer task itself is left un-awaited. Wrapping the drain section in `try/except asyncio.CancelledError` (re-raising after awaiting the task) or guarding with `asyncio.shield(producer_task)` would close this gap.
Reviews (1): Last reviewed commit: "fix delta generic errors and s3 fnf erro..." | Re-trigger Greptile |
sakce
approved these changes
May 29, 2026
9319162 to
ecc661a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
we get generic delta errors from renamed (lowercased) columns names; we also get file not found errors in s3
Changes
avoid the delta data fusion write path by using always using mode overwrite. we delete the s3 folder anyway.
to be clear, this is a temporary fix. if we want to support incremental matviews in the future we will HAVE to
find a better way around this. probably some setting somewhere in deltalake. but if we are moving to iceberg
/ ducklake and away from delta format anyway this might not be worth digging into.
How did you test this code?
tests
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Publish to changelog?
no
Docs update
no
🤖 Agent context
gg stack
andrew/delta-fnf-errors👈 this PR