[GitHub] [airflow] vincbeck commented on a diff in pull request #28502: Migrate DagFileProcessor.manage_slas to Internal API

[GitBox <gi...@apache.org>]

vincbeck commented on code in PR #28502:
URL: https://github.com/apache/airflow/pull/28502#discussion_r1062612567


##########
airflow/dag_processing/processor.py:
##########
@@ -365,8 +365,10 @@ def __init__(self, dag_ids: list[str] | None, dag_directory: str, log: logging.L
         self._dag_directory = dag_directory
         self.dag_warnings: set[tuple[str, str]] = set()
 
+    @staticmethod
+    @internal_api_call
     @provide_session
-    def manage_slas(self, dag: DAG, session: Session = None) -> None:
+    def manage_slas(dag_folder, dag_id: str, log: logging.Logger, session: Session = NEW_SESSION) -> None:

Review Comment:
   I agree, it feels also very wrong to me but maybe with some context it will help understanding it :).
   
   All methods decorated with `@internal_api_call` are meant to be called either directly (like today) or through a new component: internal API ([see PR here](https://github.com/apache/airflow/pull/27892)). This decision is based on the the flag `database_access_isolation`. What we want to achieve here is when the methods are called directly, the behavior should be the same as today, hence the logger should be the same. However, when these methods are called through the internal API, we want the logs to be separated from the current execution. Then, in that case we want a different logger. This new logger is created by the internal API itself, [see PR here](https://github.com/apache/airflow/pull/28476).
   
   I hope it helps to understand and feel free to give me your thoughts given this context
   
   
   



-- 
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