Seems like the topic of "block 0.4.0-incubating for all major backwards-incompatible changes" deserves its own thread.
On Thu, Dec 8, 2016 at 9:33 PM, Dan Halperin <[email protected]> wrote: > I did not expect this to be merged until after we'd confirmed there were no > more major changes to be made, or that they were all "ready to go". > > Are there any? If so, we should block the next release. > > On Fri, Dec 9, 2016 at 1:58 AM, Kenneth Knowles <[email protected]> > wrote: > > > Thanks all! This has been done. > > > > On Thu, Dec 8, 2016 at 3:37 AM, Amit Sela <[email protected]> wrote: > > > > > +1 > > > > > > On Thu, Dec 8, 2016 at 1:27 PM Manu Zhang <[email protected]> > > wrote: > > > > > > > +1 > > > > > > > > Manu > > > > > > > > On Thu, Dec 8, 2016 at 2:40 PM Tyler Akidau > <[email protected] > > > > > > > wrote: > > > > > > > > > +1 > > > > > > > > > > On Thu, Dec 8, 2016 at 1:10 PM Jean-Baptiste Onofré < > [email protected] > > > > > > > > wrote: > > > > > > > > > > > +1 > > > > > > > > > > > > Regards > > > > > > JB > > > > > > > > > > > > On 12/07/2016 10:37 PM, Kenneth Knowles wrote: > > > > > > > Hi all, > > > > > > > > > > > > > > I want to bring up another major backwards-incompatible change > > > before > > > > > it > > > > > > is > > > > > > > too late, to resolve [BEAM-438]. > > > > > > > > > > > > > > Summary: Leave PInput.apply the same but rename > PTransform.apply > > to > > > > > > > PTransform.expand. I have opened [PR #1538] just for reference > > (it > > > > took > > > > > > 30 > > > > > > > seconds using IDE automated refactor) > > > > > > > > > > > > > > This change affects *PTransform authors* but does *not* affect > > > > pipeline > > > > > > > authors. > > > > > > > > > > > > > > This issue was filed a long time ago. It has been a problem > many > > > > times > > > > > > with > > > > > > > actual users since before Beam started incubating. This is what > > > goes > > > > > > wrong > > > > > > > (often): > > > > > > > > > > > > > > PCollection<Foo> input = ... > > > > > > > PTransform<PCollection<Foo>, ...> transform = ... > > > > > > > > > > > > > > transform.apply(input) > > > > > > > > > > > > > > This type checks and even looks perfectly normal. Do you see > the > > > > error? > > > > > > > > > > > > > > ... what we need the user to write is: > > > > > > > > > > > > > > input.apply(transform) > > > > > > > > > > > > > > What a confusing difference! After all, the first one > type-checks > > > and > > > > > the > > > > > > > first one is how you apply a Function or Predicate or > > > > > > SerializableFunction, > > > > > > > etc. But it is broken. With transform.apply(input) the > transform > > is > > > > not > > > > > > > registered with the pipeline at all. > > > > > > > > > > > > > > We obviously can't (and don't want to) change the most core way > > > that > > > > > > > pipeline authors use Beam, so PInput.apply (aka > > PCollection.apply) > > > > must > > > > > > > remain the same. But we do need a way to make it impossible to > > mix > > > > > these > > > > > > up. > > > > > > > > > > > > > > The simplest way I can think of is to choose a new name for the > > > other > > > > > > > method involved. Users probably won't write > > transform.expand(input) > > > > > since > > > > > > > they will never have seen it in any examples, etc. This will > just > > > > make > > > > > > > PTransform authors need to do a global rename, and the type > > system > > > > will > > > > > > > direct them to all cases so there is no silent failure > possible. > > > > > > > > > > > > > > What do you think? > > > > > > > > > > > > > > Kenn > > > > > > > > > > > > > > [BEAM-438] https://issues.apache.org/jira/browse/BEAM-438 > > > > > > > [PR #1538] https://github.com/apache/incubator-beam/pull/1538 > > > > > > > > > > > > > > p.s. there is a really amusing and confusing call chain: > > > > > > PCollection.apply > > > > > > > -> Pipeline.applyTransform -> Pipeline.applyInternal -> > > > > > > > PipelineRunner.apply -> PTransform.apply > > > > > > > > > > > > > > After this change and work to get the runner out of the loop, > it > > > > > becomes > > > > > > > PCollection.apply -> Pipeline.applyTransform -> > PTransform.expand > > > > > > > > > > > > > > > > > > > -- > > > > > > Jean-Baptiste Onofré > > > > > > [email protected] > > > > > > http://blog.nanthrax.net > > > > > > Talend - http://www.talend.com > > > > > > > > > > > > > > > > > > > > >
