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
