On Mon, Jan 30, 2017 at 7:56 PM, Dan Halperin <dhalp...@google.com> wrote:
> On Mon, Jan 30, 2017 at 5:42 PM, Eugene Kirpichov < > kirpic...@google.com.invalid> wrote: > >> Hello, >> >> The PTransform Style Guide is live >> https://beam.apache.org/contribute/ptransform-style-guide/ - a natural >> next >> step is to audit Beam libraries for compliance and file JIRAs for places >> that need to be fixed. It'd be great to finish these cleanups before >> declaring Beam stable API. >> >> Please take a look and file JIRAs / post suggestions on this thread! >> >> I think it'll also make a great source of easy and useful work for new >> contributors. >> >> Some things I remember off the top of my head: >> - TextIO, KafkaIO use coders improperly - coders should not be used as a >> general-purpose byte parsing mechanism. >> > > Can you say more about Kafka? Kafka actually exports byte[] by default, > whereas Text files are String by default. So it does not seem nearly as > egregious for Kafka as it is for Text. > > - HadoopFileSource is not packaged as a PTransform >> - Some connectors, e.g. KafkaIO, should use AutoValue for their parameter >> builders, but don't >> > > Isn't AutoValue entirely an internal implementation detail that is not > exposed(*) to users? I think this is irrelevant to a stable API. > > (*) except that it makes transforms not able to be final, which is a > regression. > > I think AutoValue use should generally be considered *very* optional. In > transforms I author, I prefer not to use AutoValue because it makes the > code more complex and less readable. > > >> - A few connectors improperly use >> - Some transforms expose their transform type as "Something.Bound" and >> "Something.Unbound", e.g. TextIO.Read.Bound - such names are banned >> > > "banned" is a strong word to use here. All of these are just > recommendations. > Which is not to say that I disagree at all -- I think we even have a JIRA open for this one somewhere. Also, thanks Eugene for all the work writing this doc -- fantastic summary of your recommendations and a great starting point for the community to add in. Dan > > >> >> I filed an umbrella JIRA https://issues.apache.org/jira/browse/BEAM-1353 >> about >> making existing Beam transforms comply with the guide - let's crowdsource >> this! >> >> Thanks. >> > >
