[jira] [Commented] (BEAM-1415) PubsubIO should comply with PTransform style guide

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

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

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

GitHub user jkff opened a pull request:

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

    [BEAM-1415] PubsubIO should comply with PTransform style guide

    https://issues.apache.org/jira/browse/BEAM-1415
    
    This is a pretty big and incompatible API change:
    
    - There's no longer a way to read/write a generic type T. Instead, there's `PubsubIO.{read,write}{Strings,Protos,PubsubMessages}`. Strings and protos are a very common case so they have shorthands. For everything else, use `PubsubMessage` and parse it yourself. In case of read, you can read them with or without attributes. This gets rid of the ugly use of Coder for decoding a message's payload (forbidden by the style guide), and since PubsubMessage is easily encodable, again the style guide also dictates to use that explicitly as the input/return type of the transforms.
    - Various methods in `PubsubIO.Read` and `Write` were renamed to be style guide compliant (e.g. `topic()` -> `fromTopic()`, `idLabel()` -> `withIdLabel()` etc.)
    - `PubsubIO.Read` and `Write` converted to AutoValue
    
    Need to add a few more tests, and most likely I broke something or another in the Dataflow worker because the ways it (ab)uses the coder and parse/format fns are quite intricate, e.g. we need to give the worker a parse fn only if attributes are required, etc (I'll test/investigate in parallel), and I'll squash some commits - but other than that this is ready for review.
    
    R: @reuvenlax 
    CC: @dhalperi 

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

    $ git pull https://github.com/jkff/incubator-beam pubsub-style

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

    https://github.com/apache/beam/pull/2634.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 #2634
    
----
commit 884a9236787eb7b661d35d32e141eaf900a6d287
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:14:08Z

    Converts PubsubIO.Read to AutoValue

commit 136c968d231dc7fb874378a6a9e4a168097c0b28
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:19:37Z

    Renames PubsubIO.Read builder methods to be style guide compliant

commit 49d48145870ffbc400f60eb014823dfc87b46994
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:34:11Z

    Converts PubsubIO.Write to AutoValue

commit 752503a1742bb8993d385523971093c4fd152fb1
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:37:01Z

    PubsubIO.Read javadoc fixes

commit dfe5a8dc3b9aa85c19c8cf96bd39f2267a40c9a7
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:39:10Z

    PubsubIO.Write javadoc fixes

commit 8b2e1c395fd6ad8db6ec793c3f06a8f3df164bf7
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:41:48Z

    Renames PubsubIO.Write builder methods to be style guide compliant

commit 6f9bd45ca879bc1016f36cb3dd152ac3243d38bd
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:50:43Z

    Adds PubsubIO.readStrings()

commit fb266ac58b6f9135cbf26785ea492004a3396ce2
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:52:24Z

    Adds PubsubIO.readProtos()

commit d42ca34a90ca0485a07e00eb5760195e13da02de
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T00:54:03Z

    Adds PubsubIO.writeStrings() and writeProtos()

commit 6f56af4c17d5f4eda231dcfc824b2ed3ca8b5f98
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T01:32:23Z

    Rename read to parseFn

commit b416a3b8fc267875162f5218da9286c94ef9db83
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T01:32:38Z

    Rename write to formatFn

commit 8bd3566c5386895fe476a2c19b20c5293802c0f6
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T02:03:27Z

    Removed coder and parseFn from PubsubIO.Read - but probably does not work yet

commit fd656b875c9aa4817aed92226f84c2ebf2df9369
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T02:27:28Z

    Removes coder and formatFn from PubsubIO.Write

commit 05ad60ef9f41694184fe1152dac490388b735fa0
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T06:06:41Z

    Introduces read/writePubsubMessages

commit 1d7623e29b14148ec6042e8a8e03de893d7b71b5
Author: Eugene Kirpichov <ki...@google.com>
Date:   2017-04-21T17:37:10Z

    Adds needsAttributes; fixes PubsubIOTest

----


> PubsubIO should comply with PTransform style guide
> --------------------------------------------------
>
>                 Key: BEAM-1415
>                 URL: https://issues.apache.org/jira/browse/BEAM-1415
>             Project: Beam
>          Issue Type: Bug
>          Components: sdk-java-gcp
>            Reporter: Eugene Kirpichov
>            Assignee: Eugene Kirpichov
>              Labels: backward-incompatible, easy, starter
>             Fix For: First stable release
>
>
> Suggested changes:
> - Rename builder methods such as .subscription(), .topic() etc. to .withSubscription, .withTopic()
> - Replace use of Coder from the API (.withCoder()) with a SerializableFunction
> - Rename .withAttributes() to something else, because it sounds like this is a function that sets attributes.
> - (optional) use AutoValue



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