Merge lp:~wallyworld/launchpad/recipe-webservice-api into lp:launchpad

Just to mention, this seems gratuitiously different to the existing api for getBuildRecords, which we could expose on your context - conceptually an IHasBuildRecords or similar. Is there a reason for this?

Hi Robert

I was implementing what the bug report (712331) asked for ie publish
ISourcePackageRecipe.getBuilds() via the web service api.

getBuildRecords() certainly is very different as you say, however this
method is exposed via the web service already.

On 22/02/11 16:54, Robert Collins wrote:
> Just to mention, this seems gratuitiously different to the existing api for getBuildRecords, which we could expose on your context - conceptually an IHasBuildRecords or similar. Is there a reason for this?

requestBuild is a classic case of a named operation, but there's no reason to publish the others as named operations.
They don't change the dataset and they don't take any arguments. Let's publish them as scoped collections and keep our public API cleaner:

recipes
builds
completed_builds
pending_builds

And a link:

last_build

To do this, define fields by these names in the interfaces, and implement them with @property method definitions that invoke the method behind the scenes.

@property
def recipes(self):
    return self.getRecipes()

Or, if possible, get rid of getRecipes() altogether, and just implement .recipes for both the internal and external APIs. So 'recipes' would contain the getRecipes() implementation instead of invoking getRecipes().

> requestBuild is a classic case of a named operation, but there's no reason to
> publish the others as named operations.
> They don't change the dataset and they don't take any arguments. Let's publish
> them as scoped collections and keep our public API cleaner:
>
> recipes
> builds
> completed_builds
> pending_builds
>
> And a link:
>
> last_build
>

Hi Leonard,

I have converted the getter methods to be properties as discussed. Tests pass and running up an instance locally works fine too. I ran some additional tests: test_person, test_hasrecipes, test_request_daily_builds, since these were changed as part of the conversion to using properties.

This looks great now. My only comment is that "sorted by 'desc(date_created), then by desc(id).'" is a bad way to describe a field. This description will be going into the HTML documentation at https://launchpad.net/+apidoc/devel.html, so you should say something understandable by external developers. Something like "Sorted in descending order of creation."

On Thu, Feb 24, 2011 at 2:18 AM, Leonard Richardson
<email address hidden> wrote:
> Review: Approve
> This looks great now. My only comment is that  "sorted by 'desc(date_created), then by desc(id).'" is a bad way to describe a field. This description will be going into the HTML documentation at https://launchpad.net/+apidoc/devel.html, so you should say something understandable by external developers. Something like "Sorted in descending order of creation."

An interesting thing about phrasing it like that, is that it makes it
obvious that the sort is inefficient: better to just sort on Desc(id)
- that is precisely the same sort because ids are allocated
sequentially, and under half the work for the DB.

>
> An interesting thing about phrasing it like that, is that it makes it
> obvious that the sort is inefficient: better to just sort on Desc(id)
> - that is precisely the same sort because ids are allocated
> sequentially, and under half the work for the DB.

Done.