[GitHub] [airflow] uranusjr commented on a change in pull request #19334: Fix failing pre-commits after making 3.7 default

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

uranusjr commented on a change in pull request #19334:
URL: https://github.com/apache/airflow/pull/19334#discussion_r749965707



##########
File path: airflow/cli/simple_table.py
##########
@@ -70,13 +70,13 @@ def print_as_plain_table(self, data: List[Dict]):
             self.print("No data found")
             return
         rows = [d.values() for d in data]
-        output = tabulate(rows, tablefmt="plain", headers=data[0].keys())
+        output = tabulate(rows, tablefmt="plain", headers=list(data[0].keys()))

Review comment:
       ```suggestion
           output = tabulate(rows, tablefmt="plain", headers=list(data[0]))
   ```

##########
File path: airflow/config_templates/default_webserver_config.py
##########
@@ -18,12 +18,12 @@
 """Default configuration for the Airflow webserver"""
 import os
 
-from airflow.www.fab_security.manager import AUTH_DB
+from airflow.www.fab_security.manager import AUTH_DB  # type: ignore
 
-# from airflow.www.fab_security.manager import AUTH_LDAP
-# from airflow.www.fab_security.manager import AUTH_OAUTH
-# from airflow.www.fab_security.manager import AUTH_OID
-# from airflow.www.fab_security.manager import AUTH_REMOTE_USER
+# from airflow.www.fab_security.manager import AUTH_LDAP  # type: ignore
+# from airflow.www.fab_security.manager import AUTH_OAUTH  # type: ignore
+# from airflow.www.fab_security.manager import AUTH_OID  # type: ignore
+# from airflow.www.fab_security.manager import AUTH_REMOTE_USER  # type: ignore

Review comment:
       Why?

##########
File path: airflow/kubernetes/kubernetes_helper_functions.py
##########
@@ -84,4 +84,8 @@ def annotations_to_key(annotations: Dict[str, str]) -> Optional[TaskInstanceKey]
             .scalar()
         )
 
+    if not run_id:
+        # TODO: Is this the right way of handling it?
+        raise ValueError("Run Id has to be defined either by `run_id` annotation "
+                         "or derived from `execution_Date")

Review comment:
       We can also just do
   
   ```python
   (run_id,) = (
       session.query(TaskInstance.run_id)
       ...
       .one()
   )
   ```
   
   and let SQLAlchemy raise when there’s not a matching task instance found (it shouldn’t happen).

##########
File path: airflow/api_connexion/endpoints/dag_endpoint.py
##########
@@ -111,15 +110,15 @@ def patch_dag(session, dag_id, update_mask=None):
 
 @security.requires_access([(permissions.ACTION_CAN_DELETE, permissions.RESOURCE_DAG)])
 @provide_session
-def delete_dag(dag_id: str, session: Session):
+def delete_dag(dag_id: str, session):

Review comment:
       I don’t think this is supposed to fail?




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