You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/11/23 04:00:02 UTC

[GitHub] [airflow] dstandish opened a new pull request, #27851: Don't log CLI actions if db not initialized

dstandish opened a new pull request, #27851:
URL: https://github.com/apache/airflow/pull/27851

   This gets rid of a potentially confusing error message.
   
   Before:
   <img width="441" alt="image" src="https://user-images.githubusercontent.com/15932138/203466778-de475387-9caa-4ca9-a56e-a3809593fab6.png">
   After:
   <img width="426" alt="image" src="https://user-images.githubusercontent.com/15932138/203466794-8450909a-9693-4caf-989b-6a54d1b024fb.png">
   


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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030027869


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   oh, no, hah 



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


[GitHub] [airflow] uranusjr commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030025505


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   No that’s not guaranteed. `args` is just whatever init args the exception receives, so
   
   ```pycon
   >>> e = Exception()
   >>> e.args
   ()
   ```



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


[GitHub] [airflow] uranusjr commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030013587


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   I don’t think `e.args` can ever be missing (it’s guaranteed by Python)



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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030027459


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   but i could still iterate that



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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030024840


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   do i even need `if e.args`?  i.e. will there always be an `e.args[0]`?



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


[GitHub] [airflow] dstandish merged pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish merged PR #27851:
URL: https://github.com/apache/airflow/pull/27851


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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030024663


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   ok cool :)  i felt a bit silly writing that but i wasn't sure and wanted to err on side of safety



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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030027633


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   so this should be fine
   ```
           error_is_ok = any(x in e.args[0] for x in expected)
           if not error_is_ok:
               logging.warning("Failed to log action %s", e)
   ```
   yeah?



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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030028499


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   ok -- updated. i think we should be good to go 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


[GitHub] [airflow] dstandish commented on a diff in pull request #27851: Don't log CLI actions if db not initialized

Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #27851:
URL: https://github.com/apache/airflow/pull/27851#discussion_r1030027869


##########
airflow/utils/cli_action_loggers.py:
##########
@@ -121,8 +123,17 @@ def default_action_log(sub_command, user, task_id, dag_id, execution_date, host_
                     }
                 ],
             )
-    except Exception as error:
-        logging.warning("Failed to log action with %s", error)
+    except (OperationalError, ProgrammingError) as e:
+        expected = [
+            '"log" does not exist',  # postgres
+            "no such table",  # sqlite
+            "log' doesn't exist",  # mysql
+            "Invalid object name 'log'",  # mssql
+        ]
+        if getattr(e, "args", None) and not any(x in e.args[0] for x in expected):

Review Comment:
   oh, no, hah (not what we're iterating)



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