On Aug 20, 2014, at 6:28 PM, Salvatore Orlando <sorla...@nicira.com> wrote:
> Some comments inline. > > Salvatore > > On 20 August 2014 17:38, Ihar Hrachyshka <ihrac...@redhat.com> wrote: > >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA512 >> >> Hi all, >> >> I've read the proposal for incubator as described at [1], and I have >> several comments/concerns/suggestions to this. >> >> Overall, the idea of giving some space for experimentation that does >> not alienate parts of community from Neutron is good. In that way, we >> may relax review rules and quicken turnaround for preview features >> without loosing control on those features too much. >> >> Though the way it's to be implemented leaves several concerns, as follows: >> >> 1. From packaging perspective, having a separate repository and >> tarballs seems not optimal. As a packager, I would better deal with a >> single tarball instead of two. Meaning, it would be better to keep the >> code in the same tree. >> >> I know that we're afraid of shipping the code for which some users may >> expect the usual level of support and stability and compatibility. >> This can be solved by making it explicit that the incubated code is >> unsupported and used on your user's risk. 1) The experimental code >> wouldn't probably be installed unless explicitly requested, and 2) it >> would be put in a separate namespace (like 'preview', 'experimental', >> or 'staging', as the call it in Linux kernel world [2]). >> >> This would facilitate keeping commit history instead of loosing it >> during graduation. >> >> Yes, I know that people don't like to be called experimental or >> preview or incubator... And maybe neutron-labs repo sounds more >> appealing than an 'experimental' subtree in the core project. Well, >> there are lots of EXPERIMENTAL features in Linux kernel that we >> actively use (for example, btrfs is still considered experimental by >> Linux kernel devs, while being exposed as a supported option to RHEL7 >> users), so I don't see how that naming concern is significant. >> > > I think this is the whole point of the discussion around the incubator and > the reason for which, to the best of my knowledge, no proposal has been > accepted yet. > >> >> 2. If those 'extras' are really moved into a separate repository and >> tarballs, this will raise questions on whether packagers even want to >> cope with it before graduation. When it comes to supporting another >> build manifest for a piece of code of unknown quality, this is not the >> same as just cutting part of the code into a separate >> experimental/labs package. So unless I'm explicitly asked to package >> the incubator, I wouldn't probably touch it myself. This is just too >> much effort (btw the same applies to moving plugins out of the tree - >> once it's done, distros will probably need to reconsider which plugins >> they really want to package; at the moment, those plugins do not >> require lots of time to ship them, but having ~20 separate build >> manifests for each of them is just too hard to handle without clear >> incentive). >> > > One reason instead for moving plugins out of the main tree is allowing > their maintainers to have full control over them. > If there was a way with gerrit or similars to give somebody rights to merge > code only on a subtree I probably would not even consider the option of > moving plugin and drivers away. From my perspective it's not that I don't > want them in the main tree, it's that I don't think it's fair for core team > reviewers to take responsibility of approving code that they can't fully > tests (3rd partt CI helps, but is still far from having a decent level of > coverage). > > >> >> 3. The fact that neutron-incubator is not going to maintain any stable >> branches for security fixes and major failures concerns me too. In >> downstream, we don't generally ship the latest and greatest from PyPI. >> Meaning, we'll need to maintain our own downstream stable branches for >> major fixes. [BTW we already do that for python clients.] >> >> > This is a valid point. We need to find an appropriate trade off. My > thinking was that incubated projects could be treated just like client > libraries from a branch perspective. > > > >> 4. Another unclear part of the proposal is that notion of keeping >> Horizon and client changes required for incubator features in >> neutron-incubator. AFAIK the repo will be governed by Neutron Core >> team, and I doubt the team is ready to review Horizon changes (?). I >> think I don't understand how we're going to handle that. Can we just >> postpone Horizon work till graduation? >> >> > I too do not think it's a great idea, mostly because there will be horizon > bits not shipped with horizon, and not verified by horizon core team. > I think it would be ok to have horizon support for neutron incubator. It > won't be the first time that support for experimental features is added in > horizon. > > > 5. The wiki page says that graduation will require full test coverage. >> Does it mean 100% coverage in 'coverage' report? I don't think our >> existing code is even near that point, so maybe it's not fair to >> require that from graduated code. >> > > I agree that by these standards we should take the whole neutron and return > it to incubation, or probably just chuck it in the bin. > It's not a mystery that Neutron quality is well below integrated level but > let's not diverge. > > On the other hand, the fact that Neutron's code is rubbish does not > authorise the addition of further rubbish. > I see this requirement for graduation as a measure to ensure new additions > to neutron have proper quality. > In the meanwhile it will be mandatory for the neutron community to keep > working on quality, scalability and improve testing coverage. > Otherwise there will be no talk about neutron-incubator, mostly because > probably there will be no neutron. +1 If we judge ourselves by our past standards we have no hope of improving. I would agree, though, that the wiki requirement of ‘full test coverage’ is very much ambiguous. I would hope that a new feature would have sufficient testing to validate its target use cases and guard against regression, but that determination is unlikely to be as mechanical as ‘100% coverage achieved’. A more useful metric might be whether a quorum of cores believe that the value of a new feature exceeds the costs imposed by its ongoing maintenance. If a feature has insufficient or poor quality testing, the value of the feature could be seen to be not justified by the cost it would impose. > > >> A separate tree would probably be reasonable if it would be governed >> by a separate team. But as it looks now, it's still Neutron Cores who >> will do the review heavy-lifting. So I wonder why not just applying >> different review rules for patches for core and the staging subtree. >> > > This is a good point. As a neutron core I don't want to do the heavy > lifting there. I think we should define rules which allow teams to iterate > quickly while enabling the neutron core team to retain some form of control > on what goes in the incubator. I think divesting the current cores of responsibility of reviewing a separate plugin tree makes good sense - the impact of a given plugin on everything else is likely to be minimal. I think incubator is different, though. Why would we be incubating something if we didn’t anticipate that it could be merged into the main tree at some point - to the community's benefit - and wouldn’t we want it to be of sufficiently high quality so as to avoid a costly rewrite? I’m not sure how siloing incubation review efforts would support that outcome. I think cores should be responsible for both Neutron and incubator, but with a caveat. There should be different standards for incubator code. Patches should be allowed to land early and often - none of this ‘I want to see the whole feature before I merge anything’ that have proven so costly this cycle and forced any non-trivial feature to be developed monolithically. Let’s try iterating quickly in the incubator repo with the understanding that, unlike in the main tree, landing a patch doesn’t mean we are committing to support it indefinitely. With the understanding that periodic or final feature review are a necessary part of the process - and selectivity around what we accept for incubation - I would hope that we could balance the cost of our oversight with the benefits it promises. m. > > >> [1]: https://wiki.openstack.org/wiki/Network/Incubator >> [2]: >> >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/staging >> >> /Ihar >> -----BEGIN PGP SIGNATURE----- >> Version: GnuPG/MacGPG2 v2.0.22 (Darwin) >> >> iQEcBAEBCgAGBQJT9MDpAAoJEC5aWaUY1u57bYAH/0LsZonj3zVmWomUBBPriUOm >> GRoNBHq6C7BCfO7gRnQQyRd/N4jCL4Y1Dfbfv2Ypulsgf0x+ugvmzOrWm2Sa7KiS >> F3adumx+0OjJSMb5SSOxZQHpsZFjJmwtJjat9vwOYFXcCXhn8r9AgN3TPm5GyZ29 >> NPY+SQdqu+G/ZgXd94sE2+gGbx0H5nLZusJD0yiUpoNExhv4qvjHSZW1rwssb+Ac >> 3dU3LU1FqhM7UxkgnWk6AGYHfLjr5CfxXBrmikQsxXljl8Sko9DBTpKa3YtVcBX1 >> FdMWLGn13nFNasGAKHot/aRfmdfPIzN0TsjjfRstm0W1VLvvbQjLxGTQDEyey/U= >> =vdaC >> -----END PGP SIGNATURE----- >> >> _______________________________________________ >> OpenStack-dev mailing list >> OpenStack-dev@lists.openstack.org >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev