Vamsi-klu commented on PR #69845:
URL: https://github.com/apache/airflow/pull/69845#issuecomment-4999930706

   I took another pass over the updated bundle-path version. The new direction 
makes sense, but one production entry point still falls back to the behavior 
this PR is trying to remove. `sync_bag_to_db()` already has 
`dagbag.bundle_path` and uses it while building `files_parsed`, but it does not 
pass that path to `update_dag_parsing_results_in_db()`. Dag reserialization and 
the CLI Dag-loading path can therefore still reach `full_file_path()` inside 
the transaction.
   
   Could you pass `bundle_path=dagbag.bundle_path` there as well and add a 
regression test through `sync_bag_to_db` that asserts `full_file_path()` is not 
called? Coverage for an existing error with a supplied path would also be 
useful; that branch looks correct by inspection, but the current test covers 
only a new error and bypasses the production call chain.
   
   The PR title and body also need a refresh before rereview: the title is 
truncated and uses a `fix(...)` prefix, while the body still describes the 
superseded `has_listeners` implementation and the AI disclosure does not name 
the tool.
   
   ---
   Drafted-by: Codex (GPT-5); reviewed by @Vamsi-klu before posting
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to