You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ashb (via GitHub)" <gi...@apache.org> on 2025/07/31 11:03:48 UTC

[I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

ashb opened a new issue, #53971:
URL: https://github.com/apache/airflow/issues/53971

   ### Description
   
   With dynamic DAGs (either "old school" of `for dag_id in some_list` or using something like Dag Factory) it would be nice to be able to report an error to show up in the Airflow UI without loosing all the other dags in the file.
   
   Right now this is not possible as the only way of reporting an DAG error is to raise an exception, but if the exception is raise Airflow doesn't look for any DAGs in the loaded dag file/module anymore.
   
   We should add _some_ mechanism that lets these tools or dynamic dags report a list of exceptions so that they can still parse/load the valid dags but have a more natural experience for reporting errors.
   
   I don't have any concrete ideas on this, but it will likely have to be done by setting some "magic"/special variable at the module level that contains a list of exceptions. (The Dag Parsing process already iterates over all the attributes on the module looking for dags, so looking for this special var, say `__airflow_dag_parsing_errors`, wouldn't be a big lift.
   
   ### Use case/motivation
   
   _No response_
   
   ### Related issues
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [ ] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [x] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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: commits-unsubscribe@airflow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3172098573

   Sounds lime a plan, let's go for it


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3316266049

   Just a quick update: I’ve been working on this and I’m in the final stages. I plan to open a draft PR in the next couple of days.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3156709674

   @Shlomit-B Sure, lets talk about an implementation plan. What is your idea here?


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "VadapavKahaanHai (via GitHub)" <gi...@apache.org>.
VadapavKahaanHai commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-5075562823

   Is this issue still open? I want to contribute for it


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3191123857

   Option3 doesn't make sense to me -- Errors almost by definition can't be associated with a dag id because we weren't able to parse something succesfully.
   
   Looking at the datamodel for ParseImportError there are no uniqueness constraints, so we could store multiple errors per filename in the DB without changing anything I think?


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3219377614

   Sorry about the confusion, you’re right. The code currently assumes `(filename, bundle_name)` is unique, but we can adjust that and store multiple errors.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3156189607

   Hey, I would like to work on this one if that's ok


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3172025020

   > [@Shlomit-B](https://github.com/Shlomit-B) Sure, lets talk about an implementation plan. What is your idea here?
   
   Based on your suggestion about the module-level variable, I thought an implementation could be a safe wrapper API that users apply as a decorator or context manager—decorators for function-based DAG factories and context managers for inline DAG creation or loops—to isolate errors during DAG creation. This wrapper would catch exceptions per DAG, safely register valid DAGs, and collect error information consistently.
   
   Internally, the wrapper would set the special module-level variable (__airflow_dag_parsing_errors) with collected errors, which Airflow would read during DAG parsing to report them in the UI.
   
   Let me know what you think.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3172005413

   > [@Shlomit-B](https://github.com/Shlomit-B) Sure, lets talk about an implementation plan. What is your idea here?
   
   At the moment I see two main options:
   1. As you suggested, using a special module-level variable (__airflow_dag_parsing_errors). It’s simple to adopt with minimal changes.
   2. Providing an explicit safe wrapper (e.g `@safe_dag_factory` decorator, context manager, function, or a combination—we can discuss what fits best). This wrapper would catch exceptions per DAG, safely register valid DAGs, and consistently collect error info.
   
   I tend to prefer option 2, as it seems more consistent and robust. What are your thoughts?


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-3189469447

   @ashb 
   
   After digging more into the code:
   
   Regarding the decorator - I realized it would really only make sense for DAG generator functions. For regular DAGs it doesn’t fit well. The context manager will work in way more cases, so I think it’s better to just stick with that for now.
   
   For the __airflow_dag_parsing_errors part, currently `ParseImportError` stores one error per file. If multiple DAGs fail in a file, only the last error is saved. With per-DAG error handling, this won’t work. Possible approaches:
   
   1. **Single blob of errors per file** - store all DAG errors in one row, tracktrace; simple, but harder to query/filter by `dag_id`.
   2. **New `DagParsingError` table** - separate table for file vs DAG errors
   3. **Add `dag_id` to `ParseImportError`** - add `dag_id` (nullable) where File-level errors: `dag_id` is `NULL`
   
   I think option 3 is the best approach. What’s your thoughts? Would it make sense to open an AIP for this change?


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Add mechanism for DAG files to report errors without raising exception/loosing all DAGs in a file [airflow]

Posted by "Shlomit-B (via GitHub)" <gi...@apache.org>.
Shlomit-B commented on issue #53971:
URL: https://github.com/apache/airflow/issues/53971#issuecomment-4229323498

   I’m copying my comment from the closed PR over here to keep the discussion in one place. Would appreciate your thoughts on this:
   
   I looked into custom init / pre_init, but one concern is that on exception we may end up with a partially initialized DAG that is then used in the context manager body, without a clear signal of which args were successfully set.
   I explored an alternative approach to work around this by using a metaclass that wraps the `DAG` constructor and, on exception, returns a `FailedDAG` object instead. Something like:
   ```python
   class DAGMeta(type):
       def __call__(cls, *args: Any, **kwargs: Any) -> DAG | FailedDAG:
           try:
               return super().__call__(*args, **kwargs)
           except Exception as e:
               return FailedDAG(dag_id, e)
   ```  
   In both approaches, the context manager body still runs after a constructor failure. With the metaclass, however, it is explicit which object we’re working with, and the body fails fast when accessing missing attributes (with the resulting exception being handled by `DAG.__exit__`), allowing parsing to continue to the next `DAG`. For example:
   ```python
   with DAG(<error>) as dag:   # returns FailedDAG
       task1 = BashOp(...)     # fails when accessing missing DAG state
   ``` 
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org