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

Anne Gentle anne at openstack.org
Fri Jun 13 12:28:06 UTC 2014


On Fri, Jun 13, 2014 at 6: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.
>
>
At last week's TC meeting we discussed publishing an API Style Guide to
supplement https://wiki.openstack.org/wiki/APIChangeGuidelines

Diane Fleming, Mark McClain, and I are starting that. Read the TC take on
it at
http://eavesdrop.openstack.org/meetings/tc/2014/tc.2014-06-10-20.03.log.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.
>


Yes, Diane does an amazing job with this task. We I haven't identified
others dedicated mostly to API docs across all of OpenStack. That said, the
Nova team does a good job identifying pro-level API writers to help with
their API docs.

There are 65 Compute API doc bugs and my sense is this is an indicator and
accurately reflects what's wrong in both the v2 and v3 (2.1) docs.
http://bit.ly/compute-api-bugs

Assign yourself and ask anyone on the docs team if you need assistance
through IRC on #openstack-doc or the mailing list
openstack-docs at lists.openstack.org.

Before you complain about WADL, please let me know if you have another
solution for documenting extensible APIs and are interested in implementing
it across all the project's APIs. Not being snarky at all with that
request, just letting everyone know that our findings were "what isn't
broke do not fix" and "leave the docs to the doc pros" at the Summit. Would
love to find another solution but the one we have does the job.

Thanks,
Anne


>
> > 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/20140613/e80e5fe3/attachment.html>


More information about the OpenStack-dev mailing list