[openstack-dev] [Cinder][Glance] OSLO update

Roman Bogorodskiy rbogorodskiy at mirantis.com
Wed Nov 20 12:04:17 UTC 2013


I know it was brought up on the list a number of times, but...

If we're talking about storing commit ids for each module and writing
some shell scripts for that, isn't it a chance to reconsider using git
submodules?




On Wed, Nov 20, 2013 at 12:37 PM, Elena Ezhova <eezhova at mirantis.com> wrote:

>
> 20.11.2013, 06:18, "John Griffith" <john.griffith at solidfire.com>:
>
>
> On Mon, Nov 18, 2013 at 3:53 PM, Mark McLoughlin <markmc at redhat.com>
> wrote:
>
> >  On Mon, 2013-11-18 at 17:24 +0000, Duncan Thomas wrote:
> >>  Random OSLO updates with no list of what changed, what got fixed etc
> >>  are unlikely to get review attention - doing such a review is
> >>  extremely difficult. I was -2ing them and asking for more info, but
> >>  they keep popping up. I'm really not sure what the best way of
> >>  updating from OSLO is, but this isn't it.
> >  Best practice is to include a list of changes being synced, for example:
> >
> >    https://review.openstack.org/54660
> >
> >  Every so often, we throw around ideas for automating the generation of
> >  this changes list - e.g. cinder would have the oslo-incubator commit ID
> >  for each module stored in a file in git to tell us when it was last
> >  synced.
> >
> >  Mark.
> >
> >  _____________________________
>>
>> __________________
>> >  OpenStack-dev mailing list
>> >  OpenStack-dev at lists.openstack.org
>> >  http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>> Been away on vacation so I'm afraid I'm a bit late on this... but;
>>
>> I think the point Duncan is bringing up here is that there are some
>> VERY large and significant patches coming from OSLO pulls.  The DB
>> patch in particular being over 1K lines of code to a critical portion
>> of the code is a bit unnerving to try and do a review on.  I realize
>> that there's a level of trust that goes with the work that's done in
>> OSLO and synchronizing those changes across the projects, but I think
>> a few key concerns here are:
>>
>> 1. Doing huge pulls from OSLO like the DB patch here are nearly
>> impossible to thoroughly review and test.  Over time we learn a lot
>> about real usage scenarios and the database and tweak things as we go,
>> so seeing a patch set like this show up is always a bit unnerving and
>> frankly nobody is overly excited to review it.
>>
>> 2. Given a certain level of *trust* for the work that folks do on the
>> OSLO side in submitting these patches and new additions, I think some
>> of the responsibility on the review of the code falls on the OSLO
>> team.  That being said there is still the issue of how these changes
>> will impact projects *other* than Nova which I think is sometimes
>> neglected.  There have been a number of OSLO synchs pushed to Cinder
>> that fail gating jobs, some get fixed, some get abandoned, but in
>> either case it shows that there wasn't any testing done with projects
>> other than Nova (PLEASE note, I'm not referring to this particular
>> round of patches or calling any patch set out, just stating a
>> historical fact).
>>
>> 3. We need better documentation in commit messages explaining why the
>> changes are necessary and what they do for us.  I'm sorry but in my
>> opinion the answer "it's the latest in OSLO and Nova already has it"
>> is not enough of an answer in my opinion.  The patches mentioned in
>> this thread in my opinion met the minimum requirements because they at
>> least reference the OSLO commit which is great.  In addition I'd like
>> to see something to address any discovered issues or testing done with
>> the specific projects these changes are being synced to.
>>
>> I'm in no way saying I don't want Cinder to play nice with the common
>> code or to get in line with the way other projects do things but I am
>> saying that I think we have a ways to go in terms of better
>> communication here and in terms of OSLO code actually keeping in mind
>> the entire OpenStack eco-system as opposed to just changes that were
>> needed/updated in Nova.  Cinder in particular went through some pretty
>> massive DB re-factoring and changes during Havana and there was a lot
>> of really good work there but it didn't come without a cost and the
>> benefits were examined and weighed pretty heavily.  I also think that
>> some times the indirection introduced by adding some of the
>> openstack.common code is unnecessary and in some cases makes things
>> more difficult than they should be.
>>
>> I'm just not sure that we always do a very good ROI investigation or
>> risk assessment on changes, and that opinion applies to ALL changes in
>> OpenStack projects, not OSLO specific or anything else.
>>
>> All of that being said, a couple of those syncs on the list are
>> outdated.  We should start by doing a fresh pull for these and if
>> possible add some better documentation in the commit messages as to
>> the justification for the patches that would be great.  We can take a
>> closer look at the changes and the history behind them and try to get
>> some review progress made here.  Mark mentioned some good ideas
>> regarding capturing commit ID's from synchronization pulls and I'd
>> like to look into that a bit as well.
>>
>> Thanks,
>> John
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
>
> I see now that updating OSLO is a far more complex issue than it may seem
> from the first sight.
> But I would really like to do my best to make it look acceptable and
> possible to review.
>
> It seems everybody agrees that commit messages should have change logs of
> the updated files.
> It can be done by writing a shell script that will generate this logs by
> simply executing a git log command.
> But the problem is how to get the initial version of the file that is
> being updated.
>
> As Mark McLoughlin said, it may be a good idea to store OSLO commit-ids
> for each module in some file.
> This file will have to be changed every time an update is performed which
> implies changing update.py
> in oslo-incubator. The the shell script will just have to fetch a
> change-id from this file.
>
> I will try to write the script and generate logs, but I'm not sure about
> how can I put all these logs in
> a commit message because one patch usually changes more than one file and
> each file may have
> quite a long log.
>
> So I would really appreciate any comments or pieces of advice.
>
> Thanks,
> Elena
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20131120/a41c7548/attachment.html>


More information about the OpenStack-dev mailing list