I didn't know you could do `supertype=entity` and it will know that `entity` is a supertype corresponding to `o.a.b....Entity`.
I do think it makes sense to have `supertype` as a query param. I've no real objection to this going into /v1/catalog/types. In short, sounds fine to me, +1 to the changes Alex describes. Geoff On Fri, 22 Sep 2017 at 10:42 Aled Sage <aled.s...@gmail.com> wrote: > Hi Alex, all, > > Sounds good. > > I (reluctantly) agree to us adding this to `/v1/catalog/types` and > `/v1/catalog/bundles`. The reluctance is because the former is most > definitely intended as a replacement for the existing `/v1/catalog/*` so > should be thought of (and ideally communicated) as a v2. In the future, > going through a "beta" phase for new apis would be a good thing (so we > can better incorporate feedback without causing backwards compatibility > headaches for us or users). > > However, I see that that these changes are additive to `/v1`. This time > round we can add it to `/v1`, learn from it, and then later produce a > cut down `/v2beta` with this and a bunch of other changes. > > It will likely require a bit of effort server-side to support different > versions (such as `/v1` and `/v2beta`), while avoiding big code > duplication and thus maintenance headaches. > > Let's describe `/v1/catalog/types` as "beta" in the swagger descriptions > (not ideal compared to a clearer version, but still useful given that > this new API hasn't had time to get sufficient feedback). > > IMO we're *not* going to move to upgrading our CLI or UI to use > `/v1/catalog/types` until we do a big move to `/v2beta` and then `/v2`. > We don't really want different parts of the same tool calling old > `/v1/catalog` (e.g. for quick launch) and `/v1/catalog/types` for other > things. However, we will change the CLI to call things like `DELETE > /v1/catalog/bundles/...` as that is functionality we most definitely > want asap. > > --- > In conclusion - once the changes Alex describes are done, and once > low-level review of the PR is complete, then +1 from me to merge it. > > Aled > > > On 22/09/2017 10:15, Alex Heneveld wrote: > > > > Aled, Geoff, All- > > > > I see both /types and /bundles as integral parts of the catalog: you > > add and remove bundles and as a result you get types. so i'd like to > > see them added to the api together. And /types and /bundles as peers > > is more natural to me than `/catalog` returning types and ignoring > > bundles: feels good that these hang off `/catalog` rather than as > > top-level items as that is clearer for users so I will do that. > > > > As there isn't a conflict between this and the current API I don't > > think it buys anything to introduce a /v2 here. I do like the idea of > > a /v2beta which starts as a subset of /v1 and we make any breaking > > changes we've want in /v2beta -- the main things I can think of are > > locations (the built-in ones and the very old brooklyn.properties > > style), streaming API, and (we will need) pagination. Then clients > > can update a lot within the /v1 domain and the switch to /v2beta is > > minimal. But this can be done as a separate PR. > > > > Re `category` -- this is essentially the same as what supertypes > > should be. It seems that moving away from java-style names is desired > > (?) so the idea is that `entity` is the public-facing name of the java > > type, and when you ask for `supertype=entity` it will know that > > `entity` is a supertype corresponding to `o.a.b....Entity` and find > > the appropriate matches, but the same mechanism could work for `task` > > or `predicate` etc. Currently however supertype support is limited so > > we just support the BrooklynObjectTypes (I'll add javadoc) but neatly > > I don't think we need a different word. > > > > Sounds like the majority prefer subtypes as query filter only so i'll > > do that. > > > > Re singular v plural, this is an age-old REST debate but we use plural > > almost everywhere where the primary link `/things` returns a list and > > then `/things/1` returns an instance, so pretty sure we should follow > > the same pattern for `/bundles` and `/types`. I don't see any reason > > to switch to singular in /v2 but that could be discussed separately. > > > > I think this addresses all the major comments. Hopefully an > > acceptable conclusion? Thanks all for the good input. > > > > --A > > > > > > On 20/09/2017 20:46, Geoff Macartney wrote: > >> I guess I can live with that. It means that /v1/catalog is quite > >> different > >> from /v2/catalog, but I suppose a difference like that between a /v1 > >> and a > >> /v2 of an API isn't very remarkable. > >> > >> what do you say to > >> > >> /v2/catalog // list every type in the registry (most up to date > >> version) > >> /v2/catalog/server // list details of all versions of server > >> /v2/catalog/server/latest // list details of latest version of > >> server > >> /v2/catalog/server/1.2.3 // list details of server:1.2.3 > >> /v2/catalog/server/1.2.3/icon // icon of server:1.2.3 > >> > >> Query params: supertype, regex, versions, fragment: > >> > >> /v2/catalog?supertype=org.apache.brooklyn.entity.group.Cluster // all > >> known types extending Cluster, latest version only > >> > /v2/catalog?supertype=org.apache.brooklyn.entity.group.Cluster&version=all > >> > >> // all known types extending Cluster, all versions > >> /v2/catalog?regex=xxxxx // all known types matching the regex (might > >> need some hefty escaping!) > >> /v2/catalog/fragment=chef // all known types with 'chef' (case > >> insensitive) in the name > >> > >> More query params: applications, entities and friends: > >> > >> /v2/catalog?category=application > >> /v2/catalog?category=entity > >> /v2/catalog?category=policy > >> /v2/catalog?category=enricher > >> /v2/catalog?category=location > >> > >> Doubt 'category' is quite right but something appropriate. > >> > >> Geoff > >> > >> On Wed, 20 Sep 2017 at 18:37 Aled Sage <aled.s...@gmail.com> wrote: > >> > >>> Hi all, > >>> > >>> For `/subtypes` versus `/types`, it's not really about code > >>> duplication. > >>> It's that the API is unclear because it's not obvious what the > >>> difference/similarity is. If I saw two endpoints like that (without > >>> looking at the code), I'd assume they were for different things. If I > >>> saw query parameters, I'd immediately understand that it's a filtering > >>> mechanism. > >>> > >>> I think the right api name is `/catalog` (for what you've called > >>> `/types`). However, deprecating what's there and adding this at the > >>> same > >>> endpoint will likely be confusing. > >>> > >>> --- > >>> There's a pressing need for `/bundles` (so a user can delete a bundle), > >>> whereas `/types` is a nice-to-have. Do you agree? > >>> > >>> To me, the `/types` is a version 2 of the catalog (no matter what we > >>> call it). > >>> > >>> Thinking of how best to get this into the product in a way we want to > >>> support for a long time, the ideal would be to mark it "beta" and have > >>> it versioned. Other folk (e.g. GCE) do things like `/v2beta/`. I think > >>> that `/v2...` is the right solution from a process, api-design and > >>> understandability perspective. > >>> > >>> My feeling is we should defer the `/types`, adding it as part of a > >>> bigger `/v2beta` effort when we're ready to invest that time. This > >>> includes updating the UI and getting sufficient feedback from > >>> downstream > >>> projects that the new API is right. > >>> > >>> --- > >>> > >>> To Geoff's comment about the type registry being sufficient different > >>> from catalog, I'm not sure about that. Personally I just stretched what > >>> I think of as the catalog, and thus it is a catalog v2 which supports > >>> more things. > >>> > >>> I think if "catalog" disappeared, being replaced by "types", it > >>> would be > >>> more confusing than having a new v2 catalog that does more. > >>> > >>> Aled > >>> > >>> > >>> On 20/09/2017 17:37, Geoff Macartney wrote: > >>>> My two cents - > >>>> > >>>> 0. Keep /bundles API PR separate from /type and do /bundles now > >>>> 1. While I like '/catalog' for being more meaningful than '/type', I > >>> think > >>>> what the type registry does is sufficiently different from the classic > >>>> catalog that it needs a new name in the API. > >>>> 1.1. /type would be my preference to /types > >>>> 2. I'd deprecate catalog when we introduce /v2 (see later) > >>>> 3. I'd have said do /types now or very soon but put it under a > >>>> (clearly > >>>> beta) /v2; however I take Aled's point about clutter-free. Not sure > >>>> how > >>>> best otherwise to indicate its beta status; possibly /beta/type? And > >>> then > >>>> migrate to /v2/type at the later time? > >>>> 4. The only real use of /subtype is to have an easy way to say > >>>> 'listApplications', 'listEntities', 'listPolicies', 'listEnrichers', > >>>> 'listLocation'. I don't think this merits a separate endpoint, and in > >>>> particular I would put SubTypeApi.list into /type as > >>>> /type/?super=whatever. As for 'listPolicies' and friends, I would > >>>> have > >>>> that as an extra query parameter (not 'super') on /type. I don't > >>>> really > >>>> quite know what to call it! What name should we use to distinguish > >>> between > >>>> things that are Applications, Entities, Locations, Policies, and > >>> Enrichers. > >>>> G > >>>> > >>>> > >>>> > >>>> On Wed, 20 Sep 2017 at 16:31 Robert Moss <robert.m...@cloudsoft.io> > >>> wrote: > >>>>> 1) like the `/catalog` name and agree with Aled about its power to > >>>>> communicate the idea > >>>>> 2 & 3) run `/v1` and `/v2` in parallel, with the latter being > >>> experimental > >>>>> and iterative until a 1.0 release > >>>>> 4) no preference > >>>>> > >>>>> Robert > >>>>> > >>>>> On 20 September 2017 at 15:45, Alex Heneveld < > >>>>> alex.henev...@cloudsoftcorp.com> wrote: > >>>>> > >>>>>> Aled, Thomas, Mark - thanks for the good feedback. > >>>>>> > >>>>>> All - There are some good suggestions which I agree deserve > >>>>>> discussion. > >>>>>> Specific points. > >>>>>> > >>>>>> (1) Should /types and /bundles be top-level or under a /catalog > >>>>>> prefix > >>> ? > >>>>>> Thomas suggested the latter which also fits. My reason for doing > >>>>> top-level > >>>>>> is simply that most REST APIs these days seem to have more top-level > >>>>>> resources. /catalog is not necessary and in some ways it's cleaner > >>>>> having > >>>>>> separate endpoints. On the other hand the /catalog prefix is > >>>>>> nice to > >>>>>> orient consumers, as Aled says: `bundles` and `types` on their own > >>>>> aren't > >>>>>> as self-evident. And it means we could continue to have `POST > >>> /catalog` > >>>>> as > >>>>>> the main way to install. > >>>>>> > >>>>>> (2) Should we deprecate `/catalog` ? Thomas suggests maybe not > >>>>>> yet. I > >>>>>> think we should as equivalent functionality and more is available at > >>> the > >>>>>> new endpoints. (Though if we go with (1), obviously we wouldn't > >>>>> deprecate > >>>>>> the whole endpoint, just the old calls.). Also worth noting: I > >>>>>> don't > >>>>>> think we should remove the deprecated endpoint anytime soon however. > >>>>>> > >>>>>> (3) Should we switch to /v2 ? Aled has suggested we do as the > >>>>>> generic > >>>>>> `types` support is a significant shift from the old more restrictive > >>>>>> `catalog`. I don't think we should yet, however: I'd prefer to > >>>>>> make > >>>>> that > >>>>>> move when we are ready to remove all old endpoints so v2 is > >>> clutter-free. > >>>>>> To the extent v2 can look like a subset of v1 we make life easier > >>>>>> for > >>>>> users > >>>>>> and ourselves. Also there is significant work to add a /v2 endpoint > >>> and > >>>>> I > >>>>>> don't think there is benefit yet to justify this work. > >>>>>> > >>>>>> (4) Should `/subtypes` be an endpoint peer of `/types` ? It has > >>>>>> been > >>>>> noted > >>>>>> the same functionality as `/subtypes/entity` could be done with > >>>>>> `/types?super=entity` (with appropriate query paramter). My > >>>>>> reasoning > >>>>> for > >>>>>> making it a separate endpoint is that this is an activity I expect > >>> people > >>>>>> will want to do a lot, avoiding the query parameter makes for a > >>> slightly > >>>>>> nicer API. But it is repetition in the code. > >>>>>> > >>>>>> (There are some other minor issues but I think I agree with the > >>>>>> points > >>>>> made > >>>>>> there.) > >>>>>> > >>>>>> My answers: > >>>>>> > >>>>>> (1) slight preference for the `/catalog` prefix > >>>>>> (2) strong pref to deprecate old calls - they are redundant and > >>> multiple > >>>>> is > >>>>>> confusing > >>>>>> (3) strong pref to stay with `/v1` for now > >>>>>> (4) slight pref for explicit `[/catalog]/subtypes` endpoint > >>>>>> > >>>>>> Best > >>>>>> Alex > >>>>>> > >>>>>> > >>>>>> On 20 September 2017 at 12:38, Aled Sage <aled.s...@gmail.com> > >>>>>> wrote: > >>>>>> > >>>>>>> Thanks Alex. > >>>>>>> > >>>>>>> As per my comment in the PR at https://github.com/apache/broo > >>>>>>> klyn-server/pull/810#issuecomment-330824643. > >>>>>>> > >>>>>>> --- > >>>>>>> TL;DR: > >>>>>>> Given this is a big API change and given I'm suggesting a `/v2` > >>>>>>> REST > >>>>> api > >>>>>>> then I wanted to raise it on the list as well. > >>>>>>> > >>>>>>> I propose we split this PR into two. The `/bundles` part we can > >>>>>>> merge > >>>>>>> pretty quickly. However, the `/types` and `/subtypes` is too > >>>>>> controversial > >>>>>>> in my opinion - it probably deserves a `/v2/` of the REST api. > >>>>>>> > >>>>>>> We can continue detailed discussion in the PR. > >>>>>>> > >>>>>>> --- > >>>>>>> I don't want to lose the word "catalog" in the REST api - it's > >>>>>>> so good > >>>>> at > >>>>>>> getting across the meaning for users! The alternative `/type` is > >>>>>>> just > >>>>> not > >>>>>>> as good, in my opinion. > >>>>>>> > >>>>>>> The multiple endpoints of `/types` and `/subtypes` is confusing. > >>>>>>> I'd > >>>>>> model > >>>>>>> the latter as just a filter on `/type`. It would be better achieved > >>>>> with > >>>>>> an > >>>>>>> additional query parameter rather than a separate endpoint. > >>>>>>> > >>>>>>> If designing a `/v2` REST api, we could use `/catalog` instead of > >>>>>> `/type`. > >>>>>>> However, it will likely take a while to get to a stable and good > >>>>>>> `/v2` > >>>>>> api. > >>>>>>> There are other cleanup/improvements we should probably do to > >>>>>>> the REST > >>>>>> api > >>>>>>> if we're releasing a new version of it (e.g. exclude the deprecated > >>>>>> stuff, > >>>>>>> get rid of `/locations` but figure out if we really need to support > >>>>>>> locations from brooklyn.properties, find out from the community > >>>>>>> about > >>>>>> other > >>>>>>> inconsistencies or hard-to-understand parts of the api). > >>>>>>> > >>>>>>> The meaning of `GET /subtypes/application` looks completely > >>>>>>> different > >>>>>> from > >>>>>>> `GET /catalog/applications`. The latter retrieves the catalog items > >>>>>> marked > >>>>>>> as `template`, but the new api returns everything that implements > >>>>>>> `Application`. Perhaps this is an opportunity to get rid of the > >>>>> "entity" > >>>>>>> versus "template" distinction (at least in the REST api). The > >>>>>>> original > >>>>>>> meaning/intent of "template" has entirely changed / been misused! I > >>>>>> believe > >>>>>>> it was originally intended as a partially-complete YAML > >>>>>>> blueprint that > >>>>>>> someone would retrieve via the REST api, and then modify. They'd > >>>>>>> then > >>>>>> POST > >>>>>>> their .yaml file to deploy their app. It has now been used as > >>>>>>> the list > >>>>> of > >>>>>>> apps to include in a "quick launch" view. If designing a new `/v2` > >>> api, > >>>>>> I'd > >>>>>>> explicitly support a "quick launch" list and would get rid of the > >>>>>>> "template" versus "application" versus "entity" distinction in the > >>> REST > >>>>>> api > >>>>>>> (anywhere you can use an entity, you can use an app; anywhere > >>>>>>> you need > >>>>> an > >>>>>>> app then a non-app entity will be auto-wrapped in a basic-app). > >>>>>>> > >>>>>>> Thoughts? > >>>>>>> > >>>>>>> Aled > >>>>>>> > >>>>>>> > >>>>>>> On 07/09/2017 17:26, Alex Heneveld wrote: > >>>>>>> > >>>>>>>> Hi team- > >>>>>>>> > >>>>>>>> As mentioned earlier, I've been working on adding bundle > >>>>>>>> support to > >>>>> the > >>>>>>>> REST API, so we can add/remove/query bundles. And related to > >>>>>>>> this, > >>>>> and > >>>>>> the > >>>>>>>> type registry, is the ability to add arbitrary types but until now > >>>>> there > >>>>>>>> was no way to query those, so there are endpoints for types/ and > >>>>>>>> subtypes/. This is in #810 [1]. > >>>>>>>> > >>>>>>>> In brief you have: > >>>>>>>> > >>>>>>>> *GET /bundles* - list bundle summaries > >>>>>>>> *POST /bundles* - add ZIP or BOM YAML > >>>>>>>> *GET /bundles/com.acme/1.0* - get details on a specific bundle > >>>>>>>> *DELETE /bundles/com.acme/1.0* - remove a bundle > >>>>>>>> > >>>>>>>> *GET /types* - list all types (optionally filter by regex or > >>> fragment) > >>>>>>>> *GET /types/acme-entity/1.0* - get details on a specific type > >>>>>>>> *GET /subtypes**/entity* - list all entities (optionally filter by > >>>>> regex > >>>>>>>> or fragment); same for many other categories > >>>>>>>> > >>>>>>>> > >>>>>>>> A full list including arguments is shown in the PR. > >>>>>>>> > >>>>>>>> Another good thing about this besides bundle-centric management > >>>>>>>> and > >>>>>>>> deletion in particular is that it entirely replaces the "catalog/" > >>>>>> endpoint > >>>>>>>> allowing us to deprecate it. I expect we'll keep it around for a > >>>>> while > >>>>>> as > >>>>>>>> clients (the UI, CLI) still use it but we now have equivalent > >>>>>>>> methods > >>>>>> that > >>>>>>>> are better aligned to how we do things with bundles. They're also > >>>>>> quite a > >>>>>>>> bit faster so if you've gotten bored waiting for catalog to > >>>>>>>> load this > >>>>>>>> should help (when clients are updated). And one final benefit, we > >>> can > >>>>>> now > >>>>>>>> register and explore other types eg custom task types, predicates, > >>> and > >>>>>> more. > >>>>>>>> One thing to note is that we have fewer and simpler REST objects > >>> using > >>>>>>>> freeform maps where we return extended type info -- eg config on > >>>>>> entities, > >>>>>>>> policies, etc, sensors and effectors on entities. I'd like to use > >>> the > >>>>>> same > >>>>>>>> pattern for returning data on adjunct instances so that we can > >>> support > >>>>>>>> policies, enrichers, and feeds in a consistent way (removing > >>>>> duplication > >>>>>>>> there). This should tie in with Graeme's highlights work. > >>>>>>>> > >>>>>>>> Follow-on work will see the CLI updated to allow `br bundle delete > >>>>>>>> com.acme:1.0` and similar. No immediate plans to put lots of > >>>>>>>> bundle > >>>>>> info > >>>>>>>> into the UI as bundle devs are probably comfortable with the > >>>>>>>> CLI but > >>>>> if > >>>>>>>> anyone would like that speak up. I have updated UI to _show_ the > >>>>>>>> containing bundle ([2], also needs review!). > >>>>>>>> > >>>>>>>> Best > >>>>>>>> Alex > >>>>>>>> > >>>>>>>> [1] https://github.com/apache/brooklyn-server/pull/810 > >>>>>>>> [2] https://github.com/apache/brooklyn-ui/pull/48 > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On 07/09/2017 14:58, Alex Heneveld wrote: > >>>>>>>> > >>>>>>>>> +1 to this, with Thomas's suggestion of a list instead of map and > >>>>>>>>> Geoff's suggestion of doing it on adjuncts. would be nice to > >>>>>>>>> have > >>> an > >>>>>>>>> adjunct api which lets clients treat policies, enrichers, and > >>>>>>>>> feeds > >>>>> the > >>>>>>>>> same. > >>>>>>>>> > >>>>>>>>> i can see this being useful for policies to record selected > >>>>> highlights > >>>>>>>>> of their activity so a consumer doesn't have to trawl through all > >>>>>> activity > >>>>>>>>> to see what a policy has done lately. last value is a good > >>>>> compromise > >>>>>>>>> between having some info without trying to remember everything. > >>>>>> sensors on > >>>>>>>>> adjuncts could be another way -- maybe we'd move to that in > >>>>>>>>> future > >>> -- > >>>>>> but > >>>>>>>>> for now that seems overly complex. > >>>>>>>>> > >>>>>>>>> --a > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> On 07/09/2017 14:02, Thomas Bouron wrote: > >>>>>>>>> > >>>>>>>>>> Hi Graeme. > >>>>>>>>>> > >>>>>>>>>> Sounds very useful to me. Would be great to have this info > >>>>>>>>>> properly > >>>>>>>>>> formatted in the CLI and UI. > >>>>>>>>>> > >>>>>>>>>> As for the structure, I would suggest to avoid spaces in map > >>>>>>>>>> keys > >>> as > >>>>>>>>>> best > >>>>>>>>>> practice, so having either: > >>>>>>>>>> > >>>>>>>>>> [{ > >>>>>>>>>> ... > >>>>>>>>>> "highlights": { > >>>>>>>>>> "lastConfirmation": { > >>>>>>>>>> "name": "Last Confirmation", > >>>>>>>>>> "description": "sdjnfvsdjvfdjsng", > >>>>>>>>>> "time": 12345689, > >>>>>>>>>> "taskId": 1345 > >>>>>>>>>> }, > >>>>>>>>>> ... > >>>>>>>>>> } > >>>>>>>>>> }] > >>>>>>>>>> > >>>>>>>>>> or maybe even better, something like this: > >>>>>>>>>> [{ > >>>>>>>>>> ... > >>>>>>>>>> "highlights": [ > >>>>>>>>>> { > >>>>>>>>>> "name": "Last Confirmation", > >>>>>>>>>> "description": "sdjnfvsdjvfdjsng", > >>>>>>>>>> "time": 12345689, > >>>>>>>>>> "taskId": 1345 > >>>>>>>>>> }, > >>>>>>>>>> ... > >>>>>>>>>> ] > >>>>>>>>>> }] > >>>>>>>>>> > >>>>>>>>>> In terms of implementation, it would be useful to extend it to > >>> other > >>>>>>>>>> types > >>>>>>>>>> of Brooklyn Object such as enrichers, etc. Although, it looks > >>>>>>>>>> like > >>>>>> Geoff > >>>>>>>>>> has already made the same comment/suggestion. > >>>>>>>>>> > >>>>>>>>>> Cheers > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On Thu, 7 Sep 2017 at 13:30 Graeme Miller <gra...@cloudsoft.io> > >>>>>> wrote: > >>>>>>>>>> Hello, > >>>>>>>>>>> I'd like to make a change to the REST API for policies. As > >>>>>>>>>>> this is > >>>>> a > >>>>>>>>>>> REST > >>>>>>>>>>> API change I figured it would be best to flag it on the mailing > >>>>> list > >>>>>>>>>>> first > >>>>>>>>>>> in case anyone has any objections. > >>>>>>>>>>> > >>>>>>>>>>> It would be useful when consuming this API to be able to > >>>>>>>>>>> find out > >>>>>> more > >>>>>>>>>>> information about the policy. Specifically, it would be > >>>>>>>>>>> useful to > >>>>>> find > >>>>>>>>>>> out > >>>>>>>>>>> things like last action performed, last policy violation, last > >>>>>>>>>>> confirmation, what the triggers are etc. > >>>>>>>>>>> > >>>>>>>>>>> To do so, I plan to amend the REST API to include > >>>>>>>>>>> 'highlights' for > >>>>> a > >>>>>>>>>>> policy. Highlights are a map of a name to a tuple of > >>>>>>>>>>> information > >>>>>>>>>>> including > >>>>>>>>>>> description, time and task. > >>>>>>>>>>> > >>>>>>>>>>> Essentially this endpoint: > >>>>>>>>>>> "GET /applications/{application}/entities/{entity}/policies" > >>>>>>>>>>> Will now include this: > >>>>>>>>>>> [{ > >>>>>>>>>>> ... > >>>>>>>>>>> "highlights": { > >>>>>>>>>>> "Last Confirmation": { > >>>>>>>>>>> "description": "sdjnfvsdjvfdjsng", > >>>>>>>>>>> "time": 12345689, > >>>>>>>>>>> "taskId": 1345 > >>>>>>>>>>> }, > >>>>>>>>>>> ... > >>>>>>>>>>> } > >>>>>>>>>>> }] > >>>>>>>>>>> > >>>>>>>>>>> Please shout if you have any problems with this, otherwise I'll > >>>>>> submit > >>>>>>>>>>> a PR > >>>>>>>>>>> shortly with this change. > >>>>>>>>>>> > >>>>>>>>>>> Regards, > >>>>>>>>>>> Graeme Miller > >>>>>>>>>>> > >>>>>>>>>>> > >>> > > > >