pierrejeambrun commented on code in PR #63028:
URL: https://github.com/apache/airflow/pull/63028#discussion_r2906357037
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -74,22 +74,26 @@ def exception_handler(self, request: Request, exc:
IntegrityError):
for tb in traceback.format_tb(exc.__traceback__):
stacktrace += tb
- log_message = f"Error with id {exception_id}\n{stacktrace}"
+ log_message = f"Error with id {exception_id}, statement:
{exc.statement}\n{stacktrace}"
Review Comment:
When `expose_stacktrace=True`, the HTTPException message will hold the
`statement` in addition the the explicit 'statement' field.
That seems duplicated, why did we add this in the first place? (For a better
debugging experience?
##########
airflow-core/tests/unit/api_fastapi/common/test_exceptions.py:
##########
@@ -119,6 +119,104 @@ def teardown_method(self) -> None:
clear_db_runs()
clear_db_dags()
+ @pytest.mark.parametrize(
+ ("table", "expected_exception"),
+ generate_test_cases_parametrize(
Review Comment:
Why do we use `generate_test_cases_parametrize` if the error are the same
for all db dialect? Can't we use a normal test without parametrization. (since
the CI will run it for different db backend)
--
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]
