Lee-W commented on code in PR #49164:
URL: https://github.com/apache/airflow/pull/49164#discussion_r2299642110
##########
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:
nit: we probalby would like to verify the status code as well. same for
other tests
##########
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`."
Review Comment:
```suggestion
"Encoded dag object has no dag_id key. You may need to
run `airflow dags reserialize`."
```
##########
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):
+ """Test handling a real Dag deserialization error with actual
serialized Dag."""
+
+ # Create a test DAG
Review Comment:
I feel like we can make the setup and teardown part as a fixture to simplify
the test case.
```python
@pytest.fixture
def wrongly_serialized_dag(session: Session) -> None:
bundle_name = "testing"
session.add(DagBundleModel(name=bundle_name))
session.commit()
dag_id = "test_dag_for_error"
...
dag_model =
session.scalar(select(SerializedDagModel).where(SerializedDagModel.dag_id ==
dag_id))
...
SerializedDagModel.write_dag(test_dag, bundle_name=bundle_name)
yield
session.query(SerializedDagModel).filter(SerializedDagModel.dag_id ==
dag_id).delete()
session.commit()
```
##########
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:
```suggestion
def test_handle_real_dag_deserialization_error(self, session: Session)
-> None:
```
I think we can use `session` fixture instead of creating the session
everything
##########
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 few can probably be refactored using
`pytest.mark.paraparametreizemetreize`
##########
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):
+ """Test handling a real Dag deserialization error with actual
serialized Dag."""
+
+ # Create a test DAG
+ bundle_name = "testing"
+
+ # Create the bundle in the database first
+ with create_session() as session:
+ if session.query(DagBundleModel).filter(DagBundleModel.name ==
bundle_name).count() == 0:
+ session.add(DagBundleModel(name=bundle_name))
+ session.commit()
Review Comment:
Why do we need to check 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
