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 > >>>>> > >>>>> > >>> > >> > >> > > >
