brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aled Sage <>
Subject Re: REST API: bundles + types + subtypes endpoints
Date Wed, 20 Sep 2017 17:37:49 GMT
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 

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.


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 <> 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 <
>>> 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 <> wrote:
>>>> Thanks Alex.
>>>> As per my comment in the PR at
>>>> 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, 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]
>>>>> [2]
>>>>> 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
>>>>>> 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
>>>>>>> types
>>>>>>> of Brooklyn Object such as enrichers, etc. Although, it looks
>>> Geoff
>>>>>>> has already made the same comment/suggestion.
>>>>>>> Cheers
>>>>>>> On Thu, 7 Sep 2017 at 13:30 Graeme Miller <>
>>> 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
>>> find
>>>>>>>> out
>>>>>>>> things like last action performed, last policy violation,
>>>>>>>> confirmation, what the triggers are etc.
>>>>>>>> To do so, I plan to amend the REST API to include 'highlights'
>> 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
>>> submit
>>>>>>>> a PR
>>>>>>>> shortly with this change.
>>>>>>>> Regards,
>>>>>>>> Graeme Miller

View raw message