brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Moss <robert.m...@cloudsoft.io>
Subject Re: REST API: bundles + types + subtypes endpoints
Date Wed, 20 Sep 2017 15:30:51 GMT
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.heneveld@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.sage@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 <graeme@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
> >>>>>
> >>>>>
> >>>
> >>
> >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message