You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2015/06/30 16:59:04 UTC
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
GitHub user tbouron opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/729
[BROOKLYN-153] Update the REST API documentation for the catalog endpoints
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tbouron/incubator-brooklyn rest-api-docs
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/729.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #729
----
commit 08790a76b28f7e2d560c873118af0379ecb60895
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date: 2015-06-30T14:55:53Z
[BROOKLYN-153] Update the REST API documentation for the catalog endpoints
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-124008086
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-124007820
@tbouron Could you close this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-118003406
@neykov: Right, I was not aware of that. I guess I'm confused because we use two different terms internally and externally.
Anyway, is everyone ok if I change `type` by `symbolicName` then?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-117998557
`type` is for me the best word here, that's why I used it. I didn't really understand `symbolicName` at first, not to mention that symbolicName represents something unique to me, like an ID.
FYI, `symbolicName` is actually redundant as the API returns both `type` and `symbolicName` attributes (which have the same value every time)
That being said, I do agree that the `brooklyn.catalog.BrooklynCatalog` method's parameters need to reflect whatever name we choose for the API.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-117995274
Is "type" the right word for the documentation? It has too many meanings to be immediately understandable in this context. The methods the arguments are passed to use "symbolicName" (in brooklyn.catalog.BrooklynCatalog). Are either better than just "id"?
It's also worth changing the names of the parameters of the implementing methods in `CatalogResource` to match the interface.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-119898979
good to merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#discussion_r33951045
--- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java ---
@@ -202,18 +202,18 @@ public CatalogEntitySummary getEntity(
})
public CatalogEntitySummary getApplication(
@ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
- @PathParam("applicationId") String entityId) throws Exception;
+ @PathParam("applicationId") String applicationId) throws Exception;
@GET
- @Path("/applications/{applicationId}/{version}")
+ @Path("/applications/{symbolicName}/{version}")
@ApiOperation(value = "Fetch a specific version of an application's definition from the catalog", responseClass = "CatalogEntitySummary", multiValueResponse = true)
@ApiErrors(value = {
@ApiError(code = 404, reason = "Entity not found")
})
public CatalogEntitySummary getApplication(
- @ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
- @PathParam("applicationId") String entityId,
-
+ @ApiParam(name = "symbolicName", value = "The symbolic name of the application to retrieve", required = true)
+ @PathParam("symbolicName") String symbolicName,
+
--- End diff --
Strange, probably my Intellij not setup as it should be.
On Mon, 6 Jul 2015 at 16:37 Sam Corbett <no...@github.com> wrote:
> In usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java
> <https://github.com/apache/incubator-brooklyn/pull/729#discussion_r33947593>
> :
>
> > @ApiOperation(value = "Fetch a specific version of an application's definition from the catalog", responseClass = "CatalogEntitySummary", multiValueResponse = true)
> > @ApiErrors(value = {
> > @ApiError(code = 404, reason = "Entity not found")
> > })
> > public CatalogEntitySummary getApplication(
> > - @ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
> > - @PathParam("applicationId") String entityId,
> > -
> > + @ApiParam(name = "symbolicName", value = "The symbolic name of the application to retrieve", required = true)
> > + @PathParam("symbolicName") String symbolicName,
> > +
>
> I guess you're just following the existing format? Doesn't matter, will
> merge.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-brooklyn/pull/729/files#r33947593>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#discussion_r33947419
--- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java ---
@@ -180,15 +180,15 @@ public CatalogEntitySummary getEntity(
@PathParam("entityId") String entityId) throws Exception;
@GET
- @Path("/entities/{entityId}/{version}")
+ @Path("/entities/{symbolicName}/{version}")
@ApiOperation(value = "Fetch a specific version of an entity's definition from the catalog", responseClass = "CatalogEntitySummary", multiValueResponse = true)
@ApiErrors(value = {
@ApiError(code = 404, reason = "Entity not found")
})
public CatalogEntitySummary getEntity(
- @ApiParam(name = "entityId", value = "The ID of the entity or template to retrieve", required = true)
- @PathParam("entityId") String entityId,
-
+ @ApiParam(name = "symbolicName", value = "The symbolic name of the entity or template to retrieve", required = true)
+ @PathParam("symbolicName") String symbolicName,
+
--- End diff --
Surplus line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-118012999
`symbolicName` makes sense as then the urls will become `.../{symbolicName}/{version}` which is consistent with the internal representation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-118824789
`symbolicName` is the right unambiguous terminology -- good improvement
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#discussion_r33947593
--- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java ---
@@ -202,18 +202,18 @@ public CatalogEntitySummary getEntity(
})
public CatalogEntitySummary getApplication(
@ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
- @PathParam("applicationId") String entityId) throws Exception;
+ @PathParam("applicationId") String applicationId) throws Exception;
@GET
- @Path("/applications/{applicationId}/{version}")
+ @Path("/applications/{symbolicName}/{version}")
@ApiOperation(value = "Fetch a specific version of an application's definition from the catalog", responseClass = "CatalogEntitySummary", multiValueResponse = true)
@ApiErrors(value = {
@ApiError(code = 404, reason = "Entity not found")
})
public CatalogEntitySummary getApplication(
- @ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
- @PathParam("applicationId") String entityId,
-
+ @ApiParam(name = "symbolicName", value = "The symbolic name of the application to retrieve", required = true)
+ @PathParam("symbolicName") String symbolicName,
+
--- End diff --
I guess you're just following the existing format? Doesn't matter, will merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-118019190
Done. I also updated parameters' names within `CatalogResource` as @sjcorbett suggested.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#issuecomment-118001096
The user-visible term for catalog item is `id`. This is what the user sets in the catalog spec. The technical term is `symbolicName` with `type` kept for backwards compatibility with existing clients (it was removed initially but we had to put it back in).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/729
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: [BROOKLYN-153] Update the REST AP...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/729#discussion_r33947427
--- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java ---
@@ -202,18 +202,18 @@ public CatalogEntitySummary getEntity(
})
public CatalogEntitySummary getApplication(
@ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
- @PathParam("applicationId") String entityId) throws Exception;
+ @PathParam("applicationId") String applicationId) throws Exception;
@GET
- @Path("/applications/{applicationId}/{version}")
+ @Path("/applications/{symbolicName}/{version}")
@ApiOperation(value = "Fetch a specific version of an application's definition from the catalog", responseClass = "CatalogEntitySummary", multiValueResponse = true)
@ApiErrors(value = {
@ApiError(code = 404, reason = "Entity not found")
})
public CatalogEntitySummary getApplication(
- @ApiParam(name = "applicationId", value = "The ID of the application to retrieve", required = true)
- @PathParam("applicationId") String entityId,
-
+ @ApiParam(name = "symbolicName", value = "The symbolic name of the application to retrieve", required = true)
+ @PathParam("symbolicName") String symbolicName,
+
--- End diff --
And here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---