xuganyu96 commented on code in PR #31836:
URL: https://github.com/apache/airflow/pull/31836#discussion_r1228518699
##########
airflow/cli/commands/db_command.py:
##########
@@ -187,9 +188,18 @@ def shell(args):
@cli_utils.action_cli(check_db=False)
-def check(_):
+def check(args):
"""Runs a check command that checks if db is available."""
- db.check()
+ retries: int = args.retry
+ retry_delay: int = args.retry_delay
+
+ for attempt in Retrying(
Review Comment:
@potiuk @utkarsharma2
Since `tenacity.retry` relies on unhandled exceptions for indicating whether
a function call (or a code block in this case) is successful or not, I've
reverted the implementation of `airflow.utils.db.check` back to its original
state (letting the SQLAlchemy session raise unhandled OperationalError or
else). Consequently, there is no longer any need to unit test this util
function.
I chose to set `reraise=True` so that the original error from
`airflow.utils.db.check` can be surfaced. Please advise whether we should raise
`RetryError` or the original error. Thank you.
Last but not least, the unit test for `db_command.check` is updated
accordingly. I am not 100% sure if testing the retry logic implemented in
`tenacity.retry` is all that helpful, but the test itself still mocks
`time.sleep` and `utils.db.check` and test that they are called for the correct
number of times.
--
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]
