kennknowles commented on code in PR #17818:
URL: https://github.com/apache/beam/pull/17818#discussion_r891658075
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileBasedSink.java:
##########
@@ -1196,7 +1196,7 @@ public static <DestinationT>
FileResultCoder<DestinationT> of(
@Override
public List<? extends Coder<?>> getCoderArguments() {
- return Arrays.asList(windowCoder);
+ return Arrays.asList(windowCoder, destinationCoder);
Review Comment:
Yes, I think what you are describing makes sense. I do not know why we
decided to not have a generic parameter for `<WindowT>`. Now it would be
backwards-incompatible.
Note that `getComponents` is part of `StructuredCoder`. They overlap but are
different. The things in `getComponents` should be any coder that can be passed
to the constructor to change the behavior of the coder. Often there is no type
variable.
So based on
https://github.com/apache/beam/blob/e95ef975a3cb2f2562493ff2e0943d3e000376dc/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileBasedSink.java#L1187
the return value of `getComponents()` should be `windowCoder,
destinationCoder` but because of
https://github.com/apache/beam/blob/e95ef975a3cb2f2562493ff2e0943d3e000376dc/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileBasedSink.java#L1178
the return value of `getCoderArguments` should be just `destinationCoder`.
This class is actually a perfect example of why those two methods are
different!
--
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]
