[openstack-dev] [Oslo] Improving oslo-incubator update.py
Doug Hellmann
doug.hellmann at dreamhost.com
Wed Jan 15 19:45:38 UTC 2014
On Wed, Jan 15, 2014 at 1:53 PM, Brant Knudson <blk at acm.org> wrote:
>
> Here's the process I use to review a oslo-incubator merge... since I'm
> mostly reviewing keystone I'm going to use that as the project.
>
> 1) Make sure keystone master is at the latest, and that oslo-incubator is
> at the right level (the commit hash if they mentioned it or latest)
> 2) run update.py in oslo-incubator, make sure that there's changes in
> keystone (maybe we have it already from a different review)
> 3) revert changes in keystone and check out the review with git-review -d
> 4) run update.py again and make sure there's no changes in keystone -- If
> there's changes then I -1 the review, since I can't verify it was done
> correctly.
> 5) Then I go through the review and look for problems... changes that
> aren't backwards compatible, new config options for the sample config file,
> etc.
>
The first few steps of that seem like something that could be automated
with a fairly simple script.
> At no point do I care what are the different commits that are being
> brought in from oslo-incubator. If the commits are listed in the commit
> message then I feel an obligation to verify that they got the right commits
> in the message and that takes extra time for no gain.
>
That makes sense to me. The request from the summit was to include the
commits, but I think so far everyone is comfortable with backing that off
to just say the most recent hash.
Doug
>
> So I would prefer it if the message for syncs from oslo-incubator included
> the commit hash and did not include a list of changes.
>
> If generating the list of changes was automated then I'd add a step to my
> process to verify the commit message was the same.
>
> - Brant
>
>
>
> On Wed, Jan 15, 2014 at 6:44 AM, Doug Hellmann <
> doug.hellmann at dreamhost.com> wrote:
>
>>
>>
>>
>> On Wed, Jan 15, 2014 at 4:40 AM, Flavio Percoco <flavio at redhat.com>wrote:
>>
>>> On 14/01/14 16:33 -0600, Ben Nemec wrote:
>>>
>>>> On 2014-01-14 15:26, Doug Hellmann wrote:
>>>>
>>>> In the release meeting today, Russell proposed that we at least
>>>> include the
>>>> hash of the HEAD when the merge is done, to indicate how far along
>>>> the oslo
>>>> changes are. More detail is obviously better.
>>>> So, let's consider this as a new policy. Does anyone foresee
>>>> issues with
>>>> making this work?
>>>>
>>>>
>>>
>>> +1 from me. Lets keep it simple. I agree with you on the fact that we
>>> should be focusing more on graduating modules from the incubator.
>>>
>>>
>>> I don't see a problem with doing that, but I'm not clear on where we're
>>>> including the hash. In the file itself, in a separate file, and/or in
>>>> the
>>>> commit message?
>>>>
>>>> Even if we do no more automation, having the commit hash of the last
>>>> sync would
>>>> be immensely helpful. Not having to comb through commit logs to figure
>>>> out
>>>> when the last sync happened would be fantastic. :-)
>>>>
>>>
>>> We should keep it in the openstack-modules.conf file and put it in the
>>> commit message as well. IMHO.
>>>
>>
>> I was thinking the commit message, but if you see usefulness in including
>> the conf file, we could do that, too.
>>
>> Doug
>>
>>
>>
>>>
>>> FF
>>>
>>>
>>>
>>>> -Ben
>>>>
>>>>
>>>>
>>>> On Tue, Jan 14, 2014 at 4:23 AM, Flavio Percoco <flavio at redhat.com>
>>>> wrote:
>>>>
>>>> On 13/01/14 12:07 -0500, Doug Hellmann wrote:
>>>> [.................]
>>>>
>>>>
>>>>
>>>> I spent some time trying to think through how we could
>>>> improve the
>>>> update
>>>> script for [1], and I'm stumped on how to figure out
>>>> *accurately*
>>>> what state
>>>> the project repositories are in today.
>>>>
>>>> We can't just compute the hash of the modules in the project
>>>> receiving copies,
>>>> and then look for them in the oslo-incubator repo, because we
>>>> modify the files
>>>> as we copy them out (to update the import statements and
>>>> replace
>>>> "oslo" with
>>>> the receiving project name in some places like config option
>>>> defaults).
>>>>
>>>> We could undo those changes before computing the hash, but
>>>> the
>>>> problem is
>>>> further complicated because syncs are not being done of all
>>>> modules
>>>> together.
>>>> The common code in a project doesn't move forward in step
>>>> with the
>>>> oslo-incubator repository as a whole. For example, sometimes
>>>> only
>>>> the openstack
>>>> /common/log.py module is copied and not all of
>>>> openstack/common. So
>>>> log.py
>>>> might be newer than a lot of the rest of the oslo code. The
>>>> problem
>>>> is even
>>>> worse for something like rpc, where it's possible that
>>>> modules
>>>> within the rpc
>>>> package might not all be updated together.
>>>>
>>>> We could probably spend a lot of effort building a tool to
>>>> tell us
>>>> exactly what
>>>> the state of all of each common file is in each project, to
>>>> figure
>>>> out what
>>>> needs to be synced. I would much rather spend that effort on
>>>> turning the common
>>>> code into libraries, though.
>>>>
>>>> So, here's an alternative:
>>>>
>>>> 1. Projects accept a full sync of Oslo soon, including
>>>> adding a
>>>> value in their
>>>> openstack-common.conf indicating which commit in
>>>> oslo-incubator is
>>>> reflected in
>>>> the sync. We'll try to make those commit messages as
>>>> detailed as
>>>> possible.
>>>>
>>>> 2. We modify update.py to remove the option to update
>>>> individual
>>>> modules when
>>>> copying from oslo-incubator. The new version would always
>>>> apply all
>>>> changes
>>>> from the last merged commit, as a series of commits, to the
>>>> receiving project.
>>>> So if nova is out of step by 3 commits, then 3 new commits
>>>> would be
>>>> created in
>>>> the branch by the person doing the update, each with the
>>>> commit log
>>>> message
>>>> from the change in oslo-incubator. (This lock-step approach
>>>> is
>>>> necessary to
>>>> have any hope of figuring out which commits are actually
>>>> being
>>>> synced, so the
>>>> log messages are accurate.)
>>>>
>>>> In my experience, when syncing files from oslo, it'll most likely
>>>> require syncing more than one module. There's been just *few*
>>>> times
>>>> where copying a module from oslo resulted in just that specific
>>>> module
>>>> being copied.
>>>>
>>>> All that to say that I agree with this point.
>>>>
>>>>
>>>>
>>>>
>>>> 3. The person proposing the merge into the project can decide
>>>> whether to squash
>>>> the commits, or leave them as separate reviews.
>>>>
>>>>
>>>>
>>>>
>>>> If we use relative imports for modules in oslo incubators (as
>>>> mentioned in another email in this thread) and we *always* keep
>>>> everything up to the latest. What about reconsidering using git
>>>> submodules?
>>>>
>>>> AFAIR, the main issue with git submodules is that we wanted to
>>>> support
>>>> updating individual modules. If we remove that option, I think
>>>> git
>>>> submodules would work just fine. Am I missing something?
>>>>
>>>> Instead of hacking on update.py we could work on a migration
>>>> plan out
>>>> of it.
>>>>
>>>> A downside of using submodules is that when moving the reference
>>>> in
>>>> the submodule, it won't be obvious why that's happening, which is
>>>> something we wanted to fix with update.py. It would be up to the
>>>> committer to write a good commit message or to get the messages
>>>> out of
>>>> the submodule history.
>>>>
>>>> Another downside is that it would be hard to apply isolated
>>>> patches on
>>>> stable branches for security issues or really awful bugs.
>>>>
>>>> I'm less convinced about submodules now but I'm leaving this in
>>>> the
>>>> email in case someone wants to dig a bit deeper in the topic.
>>>>
>>>>
>>>>
>>>> I'm not entirely certain I like this approach myself, but
>>>> it's the
>>>> best I've
>>>> been able to come up with. It essentially gives us the
>>>> current
>>>> process, while
>>>> removing the ability to potentially take a version of a
>>>> module
>>>> without taking
>>>> its dependencies (allowing us to step forward, and track the
>>>> commit
>>>> messages
>>>> accurately). It will also produce results similar to what we
>>>> will
>>>> have when all
>>>> of this oslo code moves into separate libraries, where the
>>>> changes
>>>> to the
>>>> library will be seen by the projects without any action at
>>>> all on
>>>> their part.
>>>>
>>>> After going through this for a bit, I agree with you. The goal
>>>> of the update script should be:
>>>>
>>>> - Sync modules from the current state to the most updated
>>>> version
>>>>
>>>> - Make sure the update information is not lost. If there's an
>>>> oslo
>>>> sync review without the commits shas + description, it
>>>> simply
>>>> means the committer amended the message.
>>>>
>>>>
>>>>
>>>> OTOH, it will also require spending time on update.py,
>>>> instead of
>>>> releasing a
>>>> library from the incubator. And it doesn't really buy us
>>>> that much
>>>> in terms of
>>>> making the sync happen more easily, other than a reliable
>>>> way of
>>>> having
>>>> entirely accurate commit messages.
>>>>
>>>> Although it distracts us from our real goal - releasing
>>>> libraries - I
>>>> still think is necessary. We should probably just reduce the
>>>> changes
>>>> needed as much as possible, but we'll need it anyway.
>>>>
>>>>
>>>>
>>>> I would love to have someone else offer an alternative
>>>> that's less
>>>> effort to
>>>> change and provides the desired detailed log messages
>>>> accurately.
>>>>
>>>> I think this actually simplifies the way update.py works, TBH.
>>>> We'll
>>>> be removing single module sync and we'll also force projects to
>>>> be
>>>> updated to the latest version of those modules in oslo-incubator,
>>>> which is safer, IMHO.
>>>>
>>>>
>>>> Cheers,
>>>> FF
>>>>
>>>> --
>>>> @flaper87
>>>> Flavio Percoco
>>>>
>>>> _______________________________________________
>>>> OpenStack-dev mailing list
>>>> OpenStack-dev at lists.openstack.org
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/
>>>> openstack-dev
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> OpenStack-dev mailing list
>>>> OpenStack-dev at lists.openstack.orghttp://lists.openstack.org/
>>>> cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>>
>>>>
>>>>
>>>>
>>> _______________________________________________
>>>> OpenStack-dev mailing list
>>>> OpenStack-dev at lists.openstack.org
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>
>>>
>>> --
>>> @flaper87
>>> Flavio Percoco
>>>
>>> _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> _______________________________________________
> 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/20140115/00f1611a/attachment.html>
More information about the OpenStack-dev
mailing list