perkss commented on a change in pull request #1464:
URL: https://github.com/apache/samza/pull/1464#discussion_r582987562
##########
File path:
samza-core/src/test/java/org/apache/samza/serializers/model/TestSamzaObjectMapper.java
##########
@@ -165,7 +165,7 @@ public void testDeserializeUnknownTaskModelField() throws
IOException {
/**
* Given a {@link ContainerModel} JSON with neither a processor-id nor a
container-id, deserialization should fail.
*/
- @Test(expected = SamzaException.class)
Review comment:
@kw2542 a question on the API here, for general use case for
SamzaException when should we use this? For example if we get a
deserialization error generally or serialization error would we always want
SamzaException to wrap the Jackson exception. Dont think this would happen now
as they are allowed to throw JsonProcessingException. Therefore this exception
is thrown inside a deserialize call so would follow the API more suitably if it
was a Json exception. Or should we add a catch up and wrap any Jackson
exception in a Samza Exception.
In the case above with the current default jackson handling it wraps the
SamzaException with further details but the root cause is still SamzaException.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
