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 <[email protected]> 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 < > [email protected]> 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 <[email protected]> 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 <[email protected]> > > 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 > > >>>>> > > >>>>> > > >>> > > >> > > >> > > > > > >
