jedcunningham commented on code in PR #22928:
URL: https://github.com/apache/airflow/pull/22928#discussion_r847827828
##########
airflow/dag_processing/processor.py:
##########
@@ -528,23 +529,34 @@ def update_import_errors(session: Session, dagbag:
DagBag) -> None:
:param session: session for ORM operations
:param dagbag: DagBag containing DAGs with import errors
"""
- # Clear the errors of the processed files
- for dagbag_file in dagbag.file_last_changed:
- session.query(errors.ImportError).filter(
- errors.ImportError.filename.startswith(dagbag_file)
- ).delete(synchronize_session="fetch")
+ last_changed = copy.copy(dagbag.file_last_changed)
Review Comment:
I can't say I like this variable name. I had to walk this whole section to
figure out what it was.
Maybe instead we name this `all_parsed_files`, keep a separate set of files
that had errors, and iterate `all_parsed_files - files_with_errors`?
##########
airflow/dag_processing/processor.py:
##########
@@ -528,23 +529,34 @@ def update_import_errors(session: Session, dagbag:
DagBag) -> None:
:param session: session for ORM operations
:param dagbag: DagBag containing DAGs with import errors
"""
- # Clear the errors of the processed files
- for dagbag_file in dagbag.file_last_changed:
- session.query(errors.ImportError).filter(
- errors.ImportError.filename.startswith(dagbag_file)
- ).delete(synchronize_session="fetch")
+ last_changed = copy.copy(dagbag.file_last_changed)
+ import_error_files = [x.filename for x in
session.query(errors.ImportError.filename).all()]
Review Comment:
Maybe `existing_import_error_files`?
--
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]
