mynameborat commented on a change in pull request #1468:
URL: https://github.com/apache/samza/pull/1468#discussion_r586148791
##########
File path:
samza-core/src/main/java/org/apache/samza/execution/JobNodeConfigurationGenerator.java
##########
@@ -351,10 +351,10 @@ private void configureSerdes(Map<String, String> configs,
Map<String, StreamEdge
private void addSerdes(KV<Serde, Serde> serdes, String streamId, Map<String,
Serde> keySerdeMap,
Map<String, Serde> msgSerdeMap) {
if (serdes != null) {
- if (serdes.getKey() != null && !(serdes.getKey() instanceof NoOpSerde)) {
+ if (serdes.getKey() != null) {
keySerdeMap.put(streamId, serdes.getKey());
}
- if (serdes.getValue() != null && !(serdes.getValue() instanceof
NoOpSerde)) {
+ if (serdes.getValue() != null) {
Review comment:
This will impact the maps being returned to the caller which can now
have multiple instances of no-op serde even though in theory all of them are
logically the same.
While this may not impact functionality, it does create redundant copies of
serialized serdes & configurations.
----------------------------------------------------------------
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]