Fix: Persist import errors from GitDagBundle to import_error table #60100
+146
−1
Conversation
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
Arunodoy18
requested review from
ephraimbuddy and
jedcunningham
as code owners
Contributor
Author
|
This issue is now addressed in the linked PR. Requesting a review whenever convenient. |
Arunodoy18
changed the title
Contributor
|
I believe it would be best to fix the issue reference in the description so that this PR can be automatically linked to it. I will link it here now: #60059 |
2 tasks
Contributor
Author
|
Thanks for pointing that out. I’ve updated the PR description to correctly reference the issue. |
ephraimbuddy
previously approved these changes
Jan 5, 2026
potiuk
approved these changes
Jan 5, 2026
The issue: Runtime errors during DAG parsing in GitDagBundle were being caught but not persisted to the import_error table, causing DAGs with errors to silently disappear from the UI instead of appearing under Import Errors. This was inconsistent with LocalDagBundle behavior. Root cause: When DAG serialization failed in _serialize_dags(), the error was stored using dag.fileloc (absolute path) instead of dag.relative_fileloc (relative path). However, DagBag stores parse-time errors with relative paths, and the update_dag_parsing_results_in_db() function expects all import errors to be keyed by (bundle_name, relative_path) tuples. This path inconsistency caused serialization errors to have absolute paths that couldn't be properly matched to their bundle context, resulting in failed DB inserts and silent failures. Changes: 1. Updated _serialize_dags() to use dag.relative_fileloc instead of dag.fileloc when storing serialization errors, ensuring consistency with parse-time errors 2. Added test_serialization_errors_use_relative_paths() to verify serialization errors use relative paths across bundle types 3. Added test_import_errors_persisted_with_relative_paths() to validate end-to-end error persistence for bundle-backed DAGs This fix ensures that all DAG errors (parse-time and serialization-time) are consistently tracked and displayed in the UI, regardless of bundle type (Git, Local, S3, GCS, etc.). Fixes: #<issue_number>
potiuk
force-pushed
the
fix/gitdagbundle-import-error-persistence
branch
from
bc6a092 to
718a059
Compare
Member
Yep. rebased it but I guess tests will still fail |
Member
|
@Arunodoy18 I am going to close your PRs -- Please review and test your changes with correct PR description. Using LLMs without those increase maintenance burdens and CI run time. Feel free to recreate focussed PRs following those guidelines. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Fixes #60059
The issue: Runtime errors during DAG parsing in GitDagBundle were being caught but not persisted to the import_error table, causing DAGs with errors to silently disappear from the UI instead of appearing under Import Errors. This was inconsistent with LocalDagBundle behavior.
Root cause: When DAG serialization failed in _serialize_dags(), the error was stored using dag.fileloc (absolute path) instead of dag.relative_fileloc (relative path). However, DagBag stores parse-time errors with relative paths, and the update_dag_parsing_results_in_db() function expects all import errors to be keyed by (bundle_name, relative_path) tuples.
This path inconsistency caused serialization errors to have absolute paths that couldn't be properly matched to their bundle context, resulting in failed DB inserts and silent failures.
Changes:
This fix ensures that all DAG errors (parse-time and serialization-time) are consistently tracked and displayed in the UI, regardless of bundle type (Git, Local, S3, GCS, etc.).
Closes #60059