fuel-dev team mailing list archive

[Bogdan Dobrelya <bdobrelia@xxxxxxxxxxxx>]

On 03/28/2014 11:21 PM, Andrew Woodward wrote:
> OK, we are slightly off topic here and I want to steer it back.
> 
> The point isn't that we _will_ track upstream the point is that if we
> don't change we _cant_ track upstream and we _cant_ review large commits
> like this. 
> 
> This new processes wont give a damn about where you took from the
> upstream, It will leave a marker for others to be able to have a chance
> in hell of making a qualified review of the module and where you took it
> from as well as separately review you changes.
> 
> Now what has occurred with https://review.openstack.org/#/c/80024/ is
> that Dimitry has reviewed it under the guise of 
> 
>     1. Create a review request with a verbatim copy of the upstream
>     module and no other related modifications. This review should also
>     contain the commit hash from the upstream repo in the commit
>     message. The review should be looked over for reasons for rejecting
>     the entire module, such as license issues. Forbearing such issues it
>     should be accepted with out requiring modifications. 
> 
> 
> Due to this review, Dimitry has found a good reason for "rejecting the
> entire module" as the point that module was pulled from has alot of
> wasted lines that the newer version replaces. It's now necessary to
> justify why we should except this crap over the newer and cleaner
> version upstream. 
> 
> Wile we should track and keep close to upstream or use only upstream, I
> know we need to take steps. This is why I only propose that we separate
> out the upstream module pull from your own development, it gives us:
> * improved view of what you want to add
> * improved view of what had to be done to "fuel" the module
> * a clear scope of review instead of blindly adding modules
> * prevents you from being the blame when the code clearly came from upstream
> * review for really bad upstream modules (license, code quality, ...)
> * a chance (not requirement) to diff our changes from the upstream and
> propose changes back upstream
> 
> For now we can just stick with improved review process. As we get better
> we can start working on upstream's more.
> 
> So to re-iterate the rules I will update on our docs / wiki
> with https://etherpad.openstack.org/p/adding_fuel_lib_modules 

Please update the https://wiki.openstack.org/wiki/Fuel for appropriate
contribution workflow. For today it is still not clear how to contribute
to the Fuel projects, e.g. Puppet manifests for Fuel-library.

Here is an another example of stuck for MongoDB support:
1) We have a verbatim copy of upstream for puppetlabs-mongodb
https://review.openstack.org/#/c/85350/
2) We have a feature team changes for Fuel as a separate patch:
https://review.openstack.org/#/c/71901/
3) Our Ceilometer dev team is not aware of this discussion - probably
because of lack of documentation at the wiki pages.


> 
> 
> On Fri, Mar 28, 2014 at 12:41 PM, Dmitry Borodaenko
> <dborodaenko@xxxxxxxxxxxx <mailto:dborodaenko@xxxxxxxxxxxx>> wrote:
> 
>     No. The fact that we failed miserably at tracking upstream so far is
>     not a valid excuse for refusing to even try:
>     http://hakanforss.files.wordpress.com/2014/03/are-you-too-busy-to-improve1.png
> 
>     Vladimir, you optimization goal is flawed: it assumes that staying
>     close to upstream is a goal in itself. It is not a cult, there are
>     important reasons why we want to track upstream.
> 
>     Yes, sometimes we find and fix bugs in the upstream code, but it is
>     much more common for upstream to find and fix its own bugs. On
>     average, we loose code quality over time, and that causes us to lose
>     velocity by wasting efforts on fixing bugs that were already addressed
>     by upstream, or bugs that were introduced because our code is poorly
>     structured and full of ugly hacks that upstream would have done (or
>     already did) better.
> 
>     Staying close to upstream and submitting your fixes to them on a
>     regular basis also makes it more likely that upstream developers would
>     be able and willing to help you. Right now we can't report bugs
>     upstream (because we've diverged too far), we can't submit patches
>     (often because we do things in a Fuel specific way that would be
>     unacceptable to upstream), so we're always on our own when we have
>     problems with any of the Puppet code in fuel-library.
> 
>     In short, assuming that we can do a better job alone than in
>     collaboration with all our upstreams is just arrogant. We're not that
>     good and there's not that many of us.
> 
>     Bogdan, your logstash commit is such an obvious illustration of the
>     above that I am very surprised that I have to explain it.
> 
>     When I said that merging this patch will increase our Puppet code base
>     by 130% I expected everybody to understand that it is nothing short of
>     a catastrophe:
>     https://stackoverflow.com/questions/184071/when-if-ever-is-number-of-lines-of-code-a-useful-metric
> 
>     When I said that it looked auto-generated I thought it would be
>     treated as a red flag to be verified, not just an observation that can
>     be ignored. You have to have very good reasons to commit
>     auto-generated code (instead of just the generator), and
>     puppet-logstash didn't have them. Even worse, it didn't even include
>     the generator script (although commit history looks as if they kept
>     using it after the initial commit).
> 
>     The fact that current version of puppet-logstash is able to do the
>     same task in merely 673 lines of Puppet code instead of 20790 is by
>     itself a MASSIVE improvement. If we commit the old version now, and
>     give up on tracking upstream, we miss this improvement and are stuck
>     maintaining an obsolete 20KLOC version for years. It will not be
>     pleasant. If at some point we give up and try to move to a newer
>     upstream version, we'll have to deal with the fact that upstream had
>     to do a full rewrite of the manifests. And we would have to spend even
>     more effort migrating to the new upstream version in the future that
>     we would now, because we'll have accumulated more Fuel specific
>     changes that will have to be redone essentially from scratch.
> 
>     I don't see what better example of needing a newer upstream version
>     you may need. There may be cases where it makes sense to postpone
>     merging latest upstream, but this is certainly not one of them.
>     Especially since it's not merged yet.
> 
> 
>     On Fri, Mar 28, 2014 at 5:36 AM, Vladimir Kuklin
>     <vkuklin@xxxxxxxxxxxx <mailto:vkuklin@xxxxxxxxxxxx>> wrote:
>     > Guys, I suggest a hybrid approach.
>     >
>     > Let's just specify what do we want. Let me write these
>     requirements down in
>     > some kind of optimization problem
>     >
>     > We want to be as close to upstream modules as possible, subject to
>     the fact
>     > that our code should satisfy all the functional requirements for
>     particular
>     > features.
>     >
>     > In this case I would assume we need the following to do:
>     >
>     > 1) For any request require from the requester to submit the
>     original module.
>     > Not the current upstream HEAD, but his head where his own development
>     > branched from.
>     > 2) Then the developer must submit the diverged code. As long as it
>     meets
>     > functional requirements, it should be OK to use it in FUEL
>     > 3) Set upstream merging task as a "stretch goal" and strive to be
>     as close
>     > to upstream as possible.
>     >
>     > There should almost no cult of "upstream" code in our project, I
>     think.
>     > Because if there is a code piece that is not working in the
>     upstream and we
>     > fix it and upstream community does not accept it, then we have
>     nothing to do
>     > but maintain our own fork. Thus, let's split requests according to
>     this
>     > approach and request diverging changes. If they work - let's merge
>     them and
>     > leave upstream merging as background task.
>     >
>     >
>     > On Fri, Mar 28, 2014 at 12:08 PM, Bogdan Dobrelya
>     <bdobrelia@xxxxxxxxxxxx <mailto:bdobrelia@xxxxxxxxxxxx>>
>     > wrote:
>     >>
>     >> On 03/19/2014 07:45 PM, Dmitry Borodaenko wrote:
>     >> > On Wed, Mar 19, 2014 at 4:56 AM, Bogdan Dobrelya
>     >> > <bdobrelia@xxxxxxxxxxxx <mailto:bdobrelia@xxxxxxxxxxxx>> wrote:
>     >> >> On 03/19/2014 01:31 PM, Dmitry Nikishov wrote:
>     >> >>> Actually, zabbix module doesn't come from any upstream repo,
>     it's our
>     >> >>> own reimplementation of the module by PL team, which was based on
>     >> >>> upstream code. In Zabbix thread it was suggested that we
>     split it into
>     >> >>> multiple commits instead of trying to push the whole thing at
>     once.
>     >> >
>     >> > It can't be both our own reimplementation and based on upstream
>     code
>     >> > at the same time. If it's not rewritten from scratch, you
>     should track
>     >> > down the original upstream version and start your commit series
>     from
>     >> > that.
>     >> >
>     >> >>> It's unclear how it will affect internally developed modules like
>     >> >>> zabbix
>     >> >>> one. Should there be any at all? Or should we make a public
>     repo with
>     >> >>> that module first and then try to include it into fuel-library?
>     >> >
>     >> > That's a very good question. Personally, I think that we should
>     prefer
>     >> > the second option most of the time: create a public repo for the
>     >> > standalone module, and use that as upstream for the copy of that
>     >> > module in fuel-library. It will make it more likely that this
>     module
>     >> > will attract other engineers who will help us find and fix bugs in
>     >> > this code, and eventually even add more new features that we'd
>     be able
>     >> > to reuse in Fuel.
>     >> >
>     >> >> It is also not quite clear how to submit these team-specific
>     changes.
>     >> >> 1) E.g. I've submitted the puppet-logstash module from
>     Apollo11 team
>     >> >> (Poland) and that module was diverged from the original upstream
>     >> >> version
>     >> >> by the team. Now we have a two diverged versions - an upstream
>     one and
>     >> >> a
>     >> >> submitted one.
>     >> >
>     >> > I think that you should present each hand-over as a separate
>     commit:
>     >> > {original upstream commit} -> {modifications from Apollo11 team} ->
>     >> > {modifications to integrate with Fuel}. If you have the commit
>     history
>     >> > from Apollo11 and it's not too long, it would be nice to have
>     all of
>     >> > it instead of squashing it all into one commit (although if
>     it's more
>     >> > than a dozen commits it might now be worth the trouble of
>     pulling each
>     >> > commit through Gerrit).
>     >> >
>     >> >> 2) Should I submit the diverged commits rebased onto the upstream
>     >> >> HEAD/stable version as a separate patchset which depends on the
>     >> >> verbatim
>     >> >> copy of HEAD/stable patchset? That could be a very bad idea,
>     because
>     >> >> rebasing might broke the module completely.
>     >> >
>     >> > Yes, I also think that would be a bad idea. Rebase is a kind of a
>     >> > change, you don't want to combine that and other changes in a
>     single
>     >> > commit, or you loose ability to distinguish what were you
>     changes and
>     >> > what changed due to rebase.
>     >> >
>     >> >> 3) Should I do the same as (2) but use the common parent
>     commit as an
>     >> >> upstream base for verbatim copy instead? Despite on no more
>     rebasing
>     >> >> needed, that is not so good idea as well, because it would also
>     >> >> complicate the  upstream sync contribution process, if any
>     planned in
>     >> >> the future.
>     >> >
>     >> > You're not going to be able to kill two rabbits with one stone
>     here.
>     >> > Either you significantly diverge from upstream, and keeping up
>     will be
>     >> > near impossible, or you keep Fuel specific changes minimal and well
>     >> > isolated, and keeping up becomes simple.
>     >> >
>     >> > a) If you diverge, the best you can do is submit all your
>     >> > non-Fuel-specific improvements to upstream (you will obviously
>     need to
>     >> > heavily modify them to decouple from Fuel specific code), and then
>     >> > periodically (e.g. once per Fuel release) merge upstream
>     changes back
>     >> > into Fuel by hand. The further you deviate, the harder this process
>     >> > becomes. It becomes even harder if you don't submit anything to
>     >> > upstream, because there will be more changes to hand-port later.
>     >> >
>     >> > b) If you can isolate Fuel specific code, keeping up with upstream
>     >> > becomes much easier. Create a fork of upstream repo on Github,
>     create
>     >> > a fuel branch in that fork, commit all changes for that module
>     to the
>     >> > fuel branch before submitting them to fuel-library. Submit non Fuel
>     >> > specific changes to upstream (keep them on your fuel branch
>     until they
>     >> > are merged). Pulling a new upstream version into fuel-library
>     becomes
>     >> > a rebase of your fuel branch of the forked upstream onto the latest
>     >> > upstream release, and then copying the result verbatim into
>     >> > fuel-library. If Fuel specific code is well isolated, that
>     rebase will
>     >> > be trivial.
>     >> >
>     >> >> 4) So, looks like the only good option is to accept changes to the
>     >> >> puppet modules which are only the sync requests from the
>     upstream (see
>     >> >> Openstack projects and Oslo) and never change them locally in
>     the Fuel?
>     >> >> But I'm afraid the Fuel puppet modules are not ready yet for such
>     >> >> dramatical changes... Looks like we need a kind of Fuel-oslo ;)
>     >> >
>     >> > I think we're very far from being able to use this approach.
>     >> >
>     >>
>     >> Dmitry, thank you - that is a good point that makes sense.
>     >> But looks like we still have a decision-blocker for introducing
>     any new
>     >> modules for puppet in Fuel-library, if one was not synced with the
>     >> upstream while in dev / PoC process (i.e. we silently follow the
>     >> rejected options (2) and (4) despite on the fact we have near to
>     90% of
>     >> puppet modules in Fuel are far more than 1 year outdated and never
>     >> synced with upstream)
>     >>
>     >> Here is an example of such decision blockers:
>     >> https://review.openstack.org/#/c/80025/
>     >> https://review.openstack.org/#/c/80024/
>     >>
>     >> Hence, lets accept we have a blocker condition in introducing puppet
>     >> modules for Fuel, lets decide how to deal with it once again and
>     follow
>     >> it in reviews as well.
> 
>     --
>     Dmitry Borodaenko
> 
>     --
>     Mailing list: https://launchpad.net/~fuel-dev
>     Post to     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>     <mailto:fuel-dev@xxxxxxxxxxxxxxxxxxx>
>     Unsubscribe : https://launchpad.net/~fuel-dev
>     More help   : https://help.launchpad.net/ListHelp
> 
> 
> 
> 
> -- 
> Andrew
> Mirantis
> Ceph community
> 
> 
> This body part will be downloaded on demand.
> 


-- 
Best regards,
Bogdan Dobrelya,
Skype #bogdando_at_yahoo.com
Irc #bogdando