[GitHub] [airflow] ashb commented on a change in pull request #13929: Parse session from positional/kwargs argument

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

ashb commented on a change in pull request #13929:
URL: https://github.com/apache/airflow/pull/13929#discussion_r570530609



##########
File path: airflow/sentry.py
##########
@@ -151,12 +151,19 @@ def enrich_errors(self, func):
             """Wrap TaskInstance._run_raw_task to support task specific tags and breadcrumbs."""
 
             @wraps(func)
-            def wrapper(task_instance, *args, session=None, **kwargs):
+            def wrapper(task_instance, *args, **kwargs):
                 # Wrapping the _run_raw_task function with push_scope to contain
                 # tags and breadcrumbs to a specific Task Instance
+
+                try:
+                    session_args_idx = find_session_idx(func)
+                    session = kwargs.get('session', args[session_args_idx])
+                except Exception:

Review comment:
       This exception is too broad

##########
File path: airflow/sentry.py
##########
@@ -151,12 +151,19 @@ def enrich_errors(self, func):
             """Wrap TaskInstance._run_raw_task to support task specific tags and breadcrumbs."""
 
             @wraps(func)
-            def wrapper(task_instance, *args, session=None, **kwargs):
+            def wrapper(task_instance, *args, **kwargs):
                 # Wrapping the _run_raw_task function with push_scope to contain
                 # tags and breadcrumbs to a specific Task Instance
+
+                try:
+                    session_args_idx = find_session_idx(func)

Review comment:
       This should be called/"cached" outside the `wrapper` so that every call doesn't need to re-calculate this.

##########
File path: airflow/utils/session.py
##########
@@ -56,6 +51,18 @@ def provide_session(func: Callable[..., RT]) -> Callable[..., RT]:
     # We don't need this anymore -- ensure we don't keep a reference to it by mistake
     del func_params

Review comment:
       ```suggestion
   ```
   
   Not needed anymore now its a separate function.




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

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