beam-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Bradshaw <rober...@google.com>
Subject Re: Add a (temporary) Portable Flink branch to the ASF repo?
Date Mon, 16 Apr 2018 22:54:22 GMT
We now have 290 commits in (bsidhom/beam/hacking-job-server -
apache/beam/master). To better get a handle on this, I created
https://docs.google.com/spreadsheets/d/1KF5n5eTq0neIqwhIkFbvRTbp0G5mqmJ9O4ywSMWtfq0/edit#gid=1955143518
; I'd ask everyone to help fill this out (creating and/or assigning JIRAS)
and propose we rebase/merge as much as possible of this back into master
(possibly guarded by a flag, as Romain states). Even just enumerating
things that will not be suitable for merging into master as-is will be
helpful.

Thanks,
Robert



On Fri, Apr 13, 2018 at 8:47 AM Kenneth Knowles <klk@google.com> wrote:

> Yea, that's a nice idea Romain. I would support trying to move code to
> master behind a flag ASAP.
>
> The thing to remember is this: if/when a feature branch moves to master it
> is too large to review all together. So the reviews to get things onto a
> feature branch need to be the same quality and clarity standard as master.
> If it is a "hack" branch then the code needs to be re-reviewed, so the
> branch itself should not plan on merging directly but instead have the code
> copied into new PRs for full review.
>
> Kenn
>
> On Thu, Apr 12, 2018 at 9:36 PM Romain Manni-Bucau <rmannibucau@gmail.com>
> wrote:
>
>> Maybe add a toggle in flinkoptions to activate it until it is tested and
>> you are happy with it? Kind of --flinkExperimental=sdf,log,... (random
>> values). This allows to hit master and continue to work on it.
>>
>> Le 13 avr. 2018 03:07, "Robert Bradshaw" <robertwb@google.com> a écrit :
>>
>> Thomas captures exactly my concerns.
>>
>> I think we should look at getting everything we can into master (at least
>> filing JIRAs, the work may take longer) and move what development we can
>> there. What remains would be a collection of hacks (mostly in the SDKs,
>> but
>> it's not a feature branch 'cause we'd never want to actually merge it in)
>> that hopefully we can whittle away (again, JIRAs should be filed for the
>> items there).
>> On Thu, Apr 12, 2018 at 5:44 PM Henning Rohde <herohde@google.com> wrote:
>>
>> > + 1 to capture in JIRA what needs to be done.
>>
>> > The simplest path forward might be to reimplement/cherrypick'n'modify
>> the
>> changes onto master directly. We would then effectively just abandon the
>> hacking branch and treat code there as a prototype. Although we would add
>> components without end2end verification initially, it would allow parallel
>> progress across the SDKs and the Flink runner. The Go SDK is also already
>> in master and can help test the migration of the Flink runner changes
>> before the other SDKs are ready.
>>
>> > On Thu, Apr 12, 2018 at 4:44 PM Thomas Weise <thw@apache.org> wrote:
>>
>> >> Strong +1 on transitioning all development to the ASF repo.
>>
>> >> I think a straight move of the hacking branch may still be problematic
>> though, because it sets the path to continue hacking vs. working towards a
>> viable milestone that other contributors can base their work off. I would
>> prefer a state that separates serious development from hacks in a way
>> where
>> the code does not overlap.
>>
>> >> Based on Ben's assessment, if most of the hacks are currently are in
>> the
>> SDK area, then perhaps we can transition everything related to job server
>> and translation to master so that it is possible to build and work on the
>> runner there and then only use  the hacked SDKs branch for demos?
>>
>> >> And maybe discuss an MVP milestone and put together a JIRA view that
>> shows what remains to be done to get there?
>>
>> >> Thanks,
>> >> Thomas
>>
>>
>> >> On Thu, Apr 12, 2018 at 4:26 PM, Holden Karau <holden@pigscanfly.ca>
>> wrote:
>>
>> >>> So I would be strongly in favour of adding it as a branch on the
>> Apache
>> repo. This way other folks are more likely to be able to help with the
>> splitting up and merging process and also while Flink forward is behind us
>> getting in the practice of doing feature branches on the ASF repo for
>> collaboration instead of personal github accounts seems like a worthy
>> goal.
>>
>> >>> On Thu, Apr 12, 2018 at 4:21 PM Robert Bradshaw <robertwb@google.com>
>> wrote:
>>
>> >>>> I suppose with the hackathon and flink forward behind us, I'm
>> thinking
>> we
>> >>>> should start shifting gears more getting what we have into master
in
>> >>>> production state and less on continuing working on a hacking branch.
>> If we
>> >>>> think it'll fairly quick there's no big need to create an official
>> branch,
>> >>>> and if it's going to be long lived perhaps we should rethink our
>> process.
>> >>>> On Thu, Apr 12, 2018 at 3:44 PM Aljoscha Krettek <
>> aljoscha@apache.org>
>> >>>> wrote:
>>
>> >>>> > I would also be in favour of adding a branch to our main repo.
A
>> random
>> >>>> branch on some personal GitHub account can seem a bit sketchy and
>> adding a
>> >>>> branch to our repo could make it more visible for people that are
>> >>>> interested.
>>
>>
>>
>> >>>> > On 12. Apr 2018, at 15:29, Ben Sidhom <sidhom@google.com>
wrote:
>>
>> >>>> > I would say that most of it is not suitable for direct merging.
>> There are
>> >>>> several reasons for this:
>>
>> >>>> > Most changes are built on upstream PRs that are either not
>> submitted
>> or
>> >>>> have been rebased before submission.
>> >>>> > There are some very hacky changes in the Python and Java SDKs
to
>> get
>> >>>> portable pipelines working. For example, hard coding certain options
>> and/or
>> >>>> baking dependencies into the SDK harness images. These need to be
>> actually
>> >>>> implemented correctly in their respective SDKs.
>> >>>> > Much of the code does not have proper tests and fails simple
lint
>> tests.
>>
>> >>>> > As a concrete example, I tried cherry-picking the changes from
>> >>>> https://github.com/bsidhom/beam/pull/46 into master. This is a
>> relatively
>> >>>> simple change, but there were so many merge conflicts that in the
end
>> it
>> >>>> was easier to just reimplement the changes atop master. More
>> importantly,
>> >>>> most changes will require refactoring before actually going in.
>>
>> >>>> > On Thu, Apr 12, 2018 at 3:16 PM, Robert Bradshaw <
>> robertwb@google.com
>>
>> >>>> wrote:
>>
>> >>>> >> How much of this is not suitable to merging into master
directly
>> (not as
>> >>>> >> is, but as separate PRs)?
>> >>>> >> On Thu, Apr 12, 2018 at 3:10 PM Ben Sidhom <sidhom@google.com>
>> wrote:
>>
>> >>>> >> > Hey all,
>>
>> >>>> >> > I've been working on a proof-of-concept portable Flink
runner
>> with some
>> >>>> >> other Beam contributors. We would like to have a point
of
>> reference
>> for
>> >>>> the
>> >>>> >> rest of the Beam community as we integrate this work into
master.
>> It
>> >>>> >> currently lives under
>> >>>> >> https://github.com/bsidhom/beam/tree/hacking-job-server.
>>
>> >>>> >> > I would suggest pulling this into the main ASF repo
under an
>> >>>> >> appropriately-named branch (flink-portable-hacking?). The
name
>> should
>> >>>> >> suggest the intention that this branch is not intended
to be
>> pulled
>> into
>> >>>> >> master as-is and that it should rather be used as a reference
for
>> now.
>>
>> >>>> >> > Thoughts?
>>
>> >>>> >> > --
>> >>>> >> > -Ben
>>
>>
>>
>>
>> >>>> > --
>> >>>> > -Ben
>>
>> >>> --
>> >>> Twitter: https://twitter.com/holdenkarau
>>
>>
>>

Mime
View raw message