Lee-W commented on code in PR #49164:
URL: https://github.com/apache/airflow/pull/49164#discussion_r2315371311
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -1932,11 +1932,26 @@ def serialize_dag(cls, dag: SdkDag) -> dict:
@classmethod
def deserialize_dag(cls, encoded_dag: dict[str, Any]) -> SerializedDAG:
"""Deserializes a DAG from a JSON object."""
- if "dag_id" not in encoded_dag:
- raise RuntimeError(
- "Encoded dag object has no dag_id key. You may need to run
`airflow dags reserialize`."
- )
+ try:
+ if "dag_id" not in encoded_dag:
+ raise RuntimeError(
+ "Encoded dag object has no dag_id key. You may need to run
`airflow dags reserialize`."
+ )
+ except RuntimeError as err:
+ dag_id = encoded_dag.get("dag_id", None)
+ raise DeserializationError(dag_id) from err
+ try:
+ return cls._deserialize_dag_internal(encoded_dag)
+ except _TimetableNotRegistered:
+ raise
+ except (ValueError, KeyError) as err:
+ dag_id = encoded_dag.get("dag_id", None)
+ raise DeserializationError(dag_id) from err
+
+ @classmethod
+ def _deserialize_dag_internal(cls, encoded_dag: dict[str, Any]) ->
SerializedDAG:
+ """Handle the main DAG deserialization logic."""
Review Comment:
```suggestion
"""Handle the main Dag deserialization logic."""
```
##########
airflow-core/tests/unit/api_fastapi/common/test_exceptions.py:
##########
@@ -282,3 +295,91 @@ def test_handle_multiple_columns_unique_constraint_error(
assert response_detail == expected_detail
assert "INSERT INTO dag_run" in actual_statement
+ assert exeinfo_response_error.value.detail == expected_exception.detail
+
+
+class TestDagErrorHandler:
+ dag_error_handler = DagErrorHandler()
+
+ def test_handle_deserialization_error_with_value_error(self):
+ error_message = "Missing Dag ID in serialized Dag"
+ deserialization_error = DeserializationError("test_dag_id")
+
+ value_error = ValueError(error_message)
+ deserialization_error.__cause__ = value_error
+
+ with pytest.raises(
+ HTTPException,
+ match=re.escape(f"An error occurred while trying to deserialize
Dag: {deserialization_error}"),
+ ):
+ self.dag_error_handler.exception_handler(None,
deserialization_error)
+
+ def test_handle_deserialization_error_with_runtime_error(self):
+ error_message = "Error during Dag serialization process"
+ deserialization_error = DeserializationError("test_dag_id")
+
+ runtime_error = RuntimeError(error_message)
+ deserialization_error.__cause__ = runtime_error
+
+ expected_exception = HTTPException(
+ status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+ detail=f"An error occurred while trying to deserialize Dag:
{deserialization_error}",
+ )
+
+ with pytest.raises(HTTPException,
match=re.escape(expected_exception.detail)):
+ self.dag_error_handler.exception_handler(None,
deserialization_error)
+
+ def test_handle_deserialization_error_with_key_error(self):
+ key = "required_field"
+ deserialization_error = DeserializationError("test_dag_id")
+
+ key_error = KeyError(key)
+ deserialization_error.__cause__ = key_error
+
+ expected_exception = HTTPException(
+ status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+ detail=f"An error occurred while trying to deserialize Dag:
{deserialization_error}",
+ )
+ with pytest.raises(HTTPException,
match=re.escape(expected_exception.detail)):
+ self.dag_error_handler.exception_handler(None,
deserialization_error)
+
+ def test_handle_real_dag_deserialization_error(self):
Review Comment:
These test comments are non-blocking. I'll merge and create another PR for
refactoring
##########
airflow-core/src/airflow/api_fastapi/common/exceptions.py:
##########
@@ -103,6 +104,18 @@ def _is_dialect_matched(self, exc: IntegrityError) -> bool:
return False
-DatabaseErrorHandlers = [
- _UniqueConstraintErrorHandler(),
-]
+class DagErrorHandler(BaseErrorHandler[DeserializationError]):
+ """Handler for Dag related errors."""
+
+ def __init__(self):
+ super().__init__(DeserializationError)
Review Comment:
As this is non-blocking, I'm going to resolve this one. We can have a follow
up PR if we still have doubt
##########
airflow-core/tests/unit/api_fastapi/common/test_exceptions.py:
##########
@@ -282,3 +295,91 @@ def test_handle_multiple_columns_unique_constraint_error(
assert response_detail == expected_detail
assert "INSERT INTO dag_run" in actual_statement
+ assert exeinfo_response_error.value.detail == expected_exception.detail
+
+
+class TestDagErrorHandler:
+ dag_error_handler = DagErrorHandler()
+
+ def test_handle_deserialization_error_with_value_error(self):
+ error_message = "Missing Dag ID in serialized Dag"
+ deserialization_error = DeserializationError("test_dag_id")
+
+ value_error = ValueError(error_message)
+ deserialization_error.__cause__ = value_error
+
+ with pytest.raises(
+ HTTPException,
Review Comment:
These test comments are non-blocking. I'll merge and create another PR for
refactoring
--
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]
