[openstack-dev] [Nova] Review guidelines for API patches

Joe Gordon joe.gordon0 at gmail.com
Mon Jun 16 20:05:10 UTC 2014


On Fri, Jun 13, 2014 at 4:18 AM, Christopher Yeoh <cbkyeoh at gmail.com> wrote:

> Hi Phil,
>
> On Fri, 13 Jun 2014 09:28:30 +0000
> "Day, Phil" <philip.day at hp.com> wrote:
> >
> > >The documentation is NOT the canonical source for the behaviour of
> > >the API, currently the code should be seen as the reference. We've
> > >run into issues before where people have tried to align code to the
> > >fit the documentation and made backwards incompatible changes
> > >(although this is not one).
> >
> > I’ve never seen this defined before – is this published as official
> > Openstack  or Nova policy ?
>
> Its not published, but not following this guideline has got us into
> trouble before because we end up making backwards incompatible changes
> to force the code to match the docs rather than the other way around.
>

Here is a patch to publish this policy.

https://review.openstack.org/#/c/100335/

The published results of this file live at:

http://docs.openstack.org/developer/nova/devref/policies.html



>
> The documentation historically has been generated manually by people
> looking at the code and writing up what they think it does. NOTE: this
> is not a reflection on the docs team - they have done an EXCELLENT job
> based on what we've been giving them (just the api samples and the
> code). Its very easy to get it wrong and its most often someone who has
> not written the code who is writing the documentation and not familiar
> with what was actually merged.
>
> > Personally I think we should be putting as much effort into reviewing
> > the API docs as we do API code so that we can say that the API docs
> > are the canonical source for behavior.    Not being able to fix bugs
> > in say input validation that escape code reviews because they break
> > backwards compatibility seems to be a weakness to me.
>
> +1 for people to go back through the v2 api docs and fix the
> documentation where it is incorrect.
>
> So our medium term goal (and this one the reasons behind wanting the
> new v2.1/v3 infrastructure with json schema etc) is to be able to
> properly automate the production of the documentation from the code. So
> there is no contradiction between the two.
>
> I agree we need to be able to fix bugs that result in backwards
> incompatible changes. v2.1microversions should allow us to do that
> cleanly as possible.
>
> Chris
>
> >
> >
> > Phil
> >
> >
> >
> > From: Christopher Yeoh [mailto:cbkyeoh at gmail.com]
> > Sent: 13 June 2014 04:00
> > To: OpenStack Development Mailing List (not for usage questions)
> > Subject: Re: [openstack-dev] [Nova] Review guidelines for API patches
> >
> > On Fri, Jun 13, 2014 at 11:28 AM, Matt Riedemann
> > <mriedem at linux.vnet.ibm.com<mailto:mriedem at linux.vnet.ibm.com>> wrote:
> >
> >
> > On 6/12/2014 5:58 PM, Christopher Yeoh wrote:
> > On Fri, Jun 13, 2014 at 8:06 AM, Michael Still
> > <mikal at stillhq.com<mailto:mikal at stillhq.com>
> > <mailto:mikal at stillhq.com<mailto:mikal at stillhq.com>>> wrote:
> >
> >     In light of the recent excitement around quota classes and the
> >     floating ip pollster, I think we should have a conversation about
> > the review guidelines we'd like to see for API changes proposed
> > against nova. My initial proposal is:
> >
> >       - API changes should have an associated spec
> >
> >
> > +1
> >
> >       - API changes should not be merged until there is a tempest
> > change to test them queued for review in the tempest repo
> >
> >
> > +1
> >
> > Chris
> >
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org<mailto:
> OpenStack-dev at lists.openstack.org>
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
> > We do have some API change guidelines here [1].  I don't want to go
> > overboard on every change and require a spec if it's not necessary,
> > i.e. if it falls into the 'generally ok' list in that wiki.  But if
> > it's something that's not documented as a supported API (so it's
> > completely new) and is pervasive (going into novaclient so it can be
> > used in some other service), then I think that warrants some spec
> > consideration so we don't miss something.
> >
> > To compare, this [2] is an example of something that is updating an
> > existing API but I don't think warrants a blueprint since I think it
> > falls into the 'generally ok' section of the API change guidelines.
> >
> > So really I see this a new feature, not a bug fix. Someone thought
> > that detail was supported when writing the documentation but it never
> > was. The documentation is NOT the canonical source for the behaviour
> > of the API, currently the code should be seen as the reference. We've
> > run into issues before where people have tried to align code to the
> > fit the documentation and made backwards incompatible changes
> > (although this is not one).
> >
> > Perhaps we need a streamlined queue for very simple API changes, but
> > I do think API changes should get more than the usual review because
> > we have to live with them for so long (short of an emergency revert
> > if we catch it in time).
> >
> > [1] https://wiki.openstack.org/wiki/APIChangeGuidelines
> > [2] https://review.openstack.org/#/c/99443/
> >
> > --
> >
> > Thanks,
> >
> > Matt Riedemann
> >
> >
> >
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org<mailto:
> 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/20140616/c489ff87/attachment.html>


More information about the OpenStack-dev mailing list