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

   I rechecked the updated bundle-path implementation. The new direction is 
better, but one production caller still does not forward the path. 
`sync_bag_to_db()` already has `dagbag.bundle_path` and uses it while 
constructing `files_parsed`, but its call to 
`update_dag_parsing_results_in_db()` omits `bundle_path`. That path can 
therefore still fall back to `ParseImportError.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 makes `full_file_path()` fail 
if called? The current test invokes the private `_update_import_errors()` 
helper directly with a supplied path, so it stays green if a production caller 
stops forwarding that path. Coverage for an existing import-error row would 
also protect the other changed listener branch.
   
   The title and body still describe the superseded `has_listeners` 
implementation, and the AI disclosure does not name the tool, so those should 
be refreshed before rereview. Only the WIP and Mergeable gates have run on this 
head; substantive CI is still needed.
   
   ---
   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