[jira] [Commented] (BEAM-2221) Make KafkaIO coder specification less awkward

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/BEAM-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16001997#comment-16001997 ] 

ASF GitHub Bot commented on BEAM-2221:
--------------------------------------

GitHub user rangadi opened a pull request:

    https://github.com/apache/beam/pull/2986

    [BEAM-2221] KafkaIO API clean up.

    
    - Removed withKeyCoder() and withValueCoder() methods. 
      - Their meaning changed
    when KafkaIO added support for Deserializers. The coders can be explicitly
    specified using withKeyDeserializerAndCoder(). This makes it explicit to
    the user Deserializer is still required and JavaDoc explains why both
    are required in addition to deserializers.
     
     - Removed 'readWithCoders()' and 'writeWithCoders()' methods. These were supposed to be utility methods to show how to use `CoderBasedKafkaDeserializer`. But they are too prominently placed and they were misused in multiple places. E.g. KafkaIOTest used VarInt coders even though the Kafka messages were plain big endian encoded ints and longs. They tests still passed!. 
       - These utility methods  are moved `CoderBasedKafkaSerializer` (this class implements both `Deserializer` and `Serializer` interface). JavaDoc includes usage examples.
    
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rangadi/incubator-beam kafkaio_cleanup

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/beam/pull/2986.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2986
    
----
commit 89a840a9291dc8de2462e4ce76293d84a24f06f0
Author: Raghu Angadi <ra...@google.com>
Date:   2017-05-09T00:14:37Z

    KafkaIO API clean up.
    Remove withKeyCoder() and withValueCoder() method. Their meaning changed
    when KafkaIO added support for Deserializers. The coders can be explicitly
    specified using withKeyDeserializerAndCoder(). This makes it explicit to
    the user Deserializer is still required and JavaDoc explains why both
    are required.

----


> Make KafkaIO coder specification less awkward
> ---------------------------------------------
>
>                 Key: BEAM-2221
>                 URL: https://issues.apache.org/jira/browse/BEAM-2221
>             Project: Beam
>          Issue Type: Improvement
>          Components: sdk-java-extensions
>            Reporter: Eugene Kirpichov
>            Assignee: Raghu Angadi
>             Fix For: 2.0.0
>
>
> readWithCoders and writeWithCoders functions are awkward because they don't emphasize enough that coders are a poor choice for interpreting wire format.
> The only reason to specify coders in KafkaIO is when coder inference from Deserializer fails. To emphasize that, let's change the API to be withKeyDeserializer(Deserializer) as the default choice and withKeyDeserializerAndCoder(Deserializer,Coder) if inference fails; likewise for value.
> Remove functions using coders to interpret wire format from the API. A common case of that is Avro and Proto - for that, introduce special helper functions, I guess like withAvro/ProtoKey/Value(...), which under the hood may be allowed to reuse Avro/ProtoCoder as a utility, but do not expose this fact.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)