bugraoz93 commented on code in PR #49929:
URL: https://github.com/apache/airflow/pull/49929#discussion_r2070717338
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -61,13 +61,10 @@ def __init__(self):
def exception_handler(self, request: Request, exc: IntegrityError):
"""Handle IntegrityError exception."""
if self._is_dialect_matched(exc):
+ error_message = self._get_user_friendly_message(exc)
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
- detail={
Review Comment:
I don't think we should change this with a string. Some tests and logic
depend on this is expected to be a dictionary, not a string, in the API and API
tests. Is there a reason for this change?
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -79,6 +76,47 @@ def _is_dialect_matched(self, exc: IntegrityError) -> bool:
return True
return False
+ def _get_user_friendly_message(self, exc: IntegrityError) -> str:
+ """Convert database error to user-friendly message."""
+ exc_orig_str = str(exc.orig)
+
+ # Handle DAG run unique constraint
+ if "dag_run.dag_id" in exc_orig_str and "dag_run.run_id" in
exc_orig_str:
+ return "A DAG run with this ID already exists. Please use a
different run ID."
+
+ # Handle task instance unique constrain
+ if "task_instance.dag_id" in exc_orig_str and "task_instance.task_id"
in exc_orig_str:
+ return "A task instance with this ID already exists. Please use a
different task ID."
+
+ # Handle DAG run logical date unique constraint
+ if "dag_run.dag_id" in exc_orig_str and "dag_run.logical_date" in
exc_orig_str:
+ return "A DAG run with this logical date already exists. Please use
a different logical date."
+
+ # Generic unique constraint message
+ return "This operation would create a duplicate entry. Please ensure
all unique fields have unique values."
+
+class _DatabaseErrorHandler(BaseErrorHandler[SQLAlchemyError]):
+ """Handler for general database errors."""
+
+ def __init__(self):
+ super().__init__(SQLAlchemyError)
+
+ def exception_handler(self, request: Request, exc: SQLAlchemyError):
+ """Handle SQLAlchemyError exception."""
+ error_message = self._get_user_friendly_message(exc)
+ raise HTTPException(
+ status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+ detail=f"{error_message}\nType: database_error"
Review Comment:
Same here, I think enhancing the text message according to cases and making
it user-friendly is a good idea, but the other two things are also useful for
debugging.
```
"statement": str(exc.statement),
"orig_error": str(exc.orig),
```
--
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]
