You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/07/28 09:26:12 UTC
[GitHub] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/782
Adds `PUT /applications/{appid}`
For deploying an app with a pre-defined app-id. This feature is marked as beta.
This has been discussed extensively on the mailing list: https://lists.apache.org/thread.html/8eb58818d3ba50cfa5d5111363258cfe8529a76d17d45dc01823e0ba@%3Cdev.brooklyn.apache.org%3E
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server adds-deploy-with-appId
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/782.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 #782
----
commit 3cd14917cfbb83c90084b50086fe5b8e61ad5cc8
Author: Aled Sage <al...@gmail.com>
Date: 2017-07-25T16:25:31Z
Adds `PUT /applications/{appid}`
For deploying an app with a pre-defined app-id.
This feature is marked as beta.
----
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
Merging now thanks @aledsage
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
@aledsage I checked that again and all worked as expected ... sorry must have been my workspace
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/782
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130055557
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -101,6 +105,14 @@
@Context
private UriInfo uriInfo;
+ /**
+ * Only lower-case letters and digits; min 10 chars; max 1024 chars.
+ *
+ * We are this extreme because some existing entity implementations rely on the entity-id format
+ * for use in hostnames, etc.
+ */
+ private final Pattern appIdPattern = Pattern.compile("[a-z0-9]{10,1022}");
--- End diff --
Should we be rejecting UUIDs?
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130619493
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java ---
@@ -110,15 +112,32 @@ public Response createFromYaml(
required = true)
String yaml);
+ @Beta
+ @PUT
+ @Path("/{application}")
+ @Consumes({"application/x-yaml",
+ // see http://stackoverflow.com/questions/332129/yaml-mime-type
+ "text/yaml", "text/x-yaml", "application/yaml"})
+ @ApiOperation(
+ value = "[BETA] Create and start a new application from YAML, with the given id",
+ response = org.apache.brooklyn.rest.domain.TaskSummary.class
+ )
+ @ApiResponses(value = {
+ @ApiResponse(code = 404, message = "Undefined entity or location"),
+ @ApiResponse(code = 409, message = "Application already registered")
--- End diff --
@m4rkmckenna weird - my test `ApplicationResourceTest.testDeploymentFailsOnDuplicateAppId` asserts that it gives back a 409. I'll investigate further (building Brooklyn, and testing via the REST api directly).
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
@m4rkmckenna I've decreased max length to 63 chars, and moved validation to `LocalEntityManager` - can you take another look please?
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130148566
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -101,6 +105,14 @@
@Context
private UriInfo uriInfo;
+ /**
+ * Only lower-case letters and digits; min 10 chars; max 1024 chars.
+ *
+ * We are this extreme because some existing entity implementations rely on the entity-id format
+ * for use in hostnames, etc.
+ */
+ private final Pattern appIdPattern = Pattern.compile("[a-z0-9]{10,1022}");
--- End diff --
Yeah, I don't like how restrictive it is - but wanted to play it safe (particularly given @neykov's observation about its use in cloud resource names, where we might not sanitize it).
I also wanted to fail-fast, so wanted to do the check early. Pushing the check into `LocalEntityManager` makes sense. Would we repeat it here still, or just do it in that one place? I lean towards just the one place - we shouldn't be causing any side-effects in the code leading up to the id being used.
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130627191
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java ---
@@ -110,15 +112,32 @@ public Response createFromYaml(
required = true)
String yaml);
+ @Beta
+ @PUT
+ @Path("/{application}")
+ @Consumes({"application/x-yaml",
+ // see http://stackoverflow.com/questions/332129/yaml-mime-type
+ "text/yaml", "text/x-yaml", "application/yaml"})
+ @ApiOperation(
+ value = "[BETA] Create and start a new application from YAML, with the given id",
+ response = org.apache.brooklyn.rest.domain.TaskSummary.class
+ )
+ @ApiResponses(value = {
+ @ApiResponse(code = 404, message = "Undefined entity or location"),
+ @ApiResponse(code = 409, message = "Application already registered")
--- End diff --
@m4rkmckenna This works for me. I ran `mvn clean install -o -DskipTests` for all brooklyn projects, then I unpacked and ran `brooklyn-dist/karaf/apache-brooklyn/target/apache-brooklyn-0.12.0-SNAPSHOT.tar.gz`, and then tried to deploy with the same id twice:
```
curl -v -X PUT -u admin:password -H "Content-Type: application/x-yaml" http://localhost:8081/v1/applications/thisismyid --data-binary @/Users/aledsage/amp-cluster/app.yaml
* Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8081 (#0)
* Server auth using Basic with user 'admin'
> PUT /v1/applications/thisismyid HTTP/1.1
> Host: localhost:8081
> Authorization: Basic YWRtaW46cGFzc3dvcmQ=
> User-Agent: curl/7.51.0
> Accept: */*
> Content-Type: application/x-yaml
> Content-Length: 69
>
* upload completely sent off: 69 out of 69 bytes
< HTTP/1.1 409 Conflict
< Date: Tue, 01 Aug 2017 14:38:34 GMT
< Content-Type: application/json
< Cache-Control: no-cache, no-store
< Pragma: no-cache
< Expires: 0
< Transfer-Encoding: chunked
< Server: Jetty(9.2.19.v20160908)
<
{"message":"Error launching blueprint, id already exists: IdAlreadyExistsException: call to manage entity BasicApplicationImpl{id=thisismyid} (ManagementTransitionMode[NONEXISTENT->MANAGED_PRIMARY]) but different entity BasicApplicationImpl{id=thisismyid} already known under that id 'thisismyid' at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager@4a88c541","details":"org.apache.brooklyn.core.mgmt.internal.IdAlreadyExistsException: call to manage entity BasicApplicationImpl{id=thisismyid} (ManagementTransitionMode[NONEXISTENT->MANAGED_PRIMARY]) but different entity BasicApplicationImpl{id=thisismyid} already known under that id 'thisismyid' at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager@4a88c541\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.manageNonRecursive(LocalEntityManager.java:701)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.access$300(LocalEntityManager.java:84)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEn
tityManager$2.apply(LocalEntityManager.java:419)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager$2.apply(LocalEntityManager.java:374)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.recursively(LocalEntityManager.java:645)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.manageRecursive(LocalEntityManager.java:439)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.manage(LocalEntityManager.java:333)\n\tat org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.createEntity(LocalEntityManager.java:177)\n\tat org.apache.brooklyn.core.mgmt.EntityManagementUtils.createUnstarted(EntityManagementUtils.java:94)\n\tat org.apache.brooklyn.rest.resources.ApplicationResource.launch(ApplicationResource.java:312)\n\tat org.apache.brooklyn.rest.resources.ApplicationResource.createFromYaml(ApplicationResource.java:299)\n\tat org.apache.brooklyn.rest.resources.ApplicationResource.createFromYamlWithAppId(ApplicationResource.java:251)\n\t
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\n\tat sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)\n\tat sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\n\tat java.lang.reflect.Method.invoke(Method.java:498)\n\tat org.apache.cxf.service.invoker.AbstractInvoker.performInvocation(AbstractInvoker.java:180)\n\tat org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:96)\n\tat org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:189)\n\tat org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:99)\n\tat org.apache.cxf.interceptor.ServiceInvokerInterceptor$1.run(ServiceInvokerInterceptor.java:59)\n\tat org.apache.cxf.interceptor.ServiceInvokerInterceptor.handleMessage(ServiceInvokerInterceptor.java:96)\n\tat org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)\n\tat org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationO
bserver.java:121)\n\tat org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:262)\n\tat org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:234)\n\tat org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:208)\n\tat org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:160)\n\tat org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:180)\n\tat org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:299)\n\tat org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPut(AbstractHTTPServlet.java:235)\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:710)\n\tat org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:274)\n\tat org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:812)\n\tat org.eclipse.jetty.servlet.ServletHandler.doHandle(
ServletHandler.java:587)\n\tat org.ops4j.pax.web.service.jetty.internal.HttpServiceServletHandler.doHandle(Http* Curl_http_done: called premature == 0
* Connection #0 to host localhost left intact
ServiceServletHandler.java:71)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)\n\tat org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:577)\n\tat org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223)\n\tat org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127)\n\tat org.ops4j.pax.web.service.jetty.internal.HttpServiceContext.doHandle(HttpServiceContext.java:287)\n\tat org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)\n\tat org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)\n\tat org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)\n\tat org.ops4j.pax.web.service.jetty.internal.JettyServerHandlerCollection.handle(JettyServerHandlerCollection.java:80)\n\tat org.eclipse.jetty.server.handler.HandlerWrapper.ha
ndle(HandlerWrapper.java:97)\n\tat org.eclipse.jetty.server.Server.handle(Server.java:499)\n\tat org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)\n\tat org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)\n\tat org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)\n\tat java.lang.Thread.run(Thread.java:745)\n","error":409}
```
It's not pretty, with the full stacktrace being included, but it does give the 409 response and does include the exception's message.
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130405178
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java ---
@@ -143,12 +153,24 @@ public EntityTypeRegistry getEntityTypeRegistry() {
if (!isRunning()) throw new IllegalStateException("Management context no longer running");
return entityTypeRegistry;
}
+
+ @Override
+ public <T extends Entity> T createEntity(EntitySpec<T> spec) {
+ return createEntity(spec, Optional.absent());
+ }
+ @Beta
@SuppressWarnings("unchecked")
@Override
- public <T extends Entity> T createEntity(EntitySpec<T> spec) {
+ public <T extends Entity> T createEntity(EntitySpec<T> spec, Optional<String> entityId) {
+ if (entityId.isPresent()) {
+ if (!ENTITY_ID_PATTERN.matcher(entityId.get()).matches()) {
+ throw new IllegalArgumentException("Invalid entity id '"+entityId.get()+"'");
--- End diff --
This exception text is never returned so its hard to diagnose the issue
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130405300
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java ---
@@ -670,23 +694,26 @@ private synchronized boolean manageNonRecursive(Entity e, ManagementTransitionMo
Entity old = entitiesById.get(e.getId());
if (old!=null && mode.wasNotLoaded()) {
- if (old.equals(e)) {
+ if (old == deproxyIfNecessary(e)) {
log.warn("{} redundant call to start management of entity {}; ignoring", this, e);
} else {
- throw new IllegalStateException("call to manage entity "+e+" ("+mode+") but different entity "+old+" already known under that id at "+this);
+ throw new IdAlreadyExistsException("call to manage entity "+e+" ("+mode+") but "
+ + "different entity "+old+" already known under that id '"+e.getId()+"' at "+this);
}
return false;
}
BrooklynLogging.log(log, BrooklynLogging.levelDebugOrTraceIfReadOnly(e),
- "{} starting management of entity {}", this, e);
+ "{} starting management of entity {}", this, e);
Entity realE = toRealEntity(e);
Entity oldProxy = entityProxiesById.get(e.getId());
Entity proxyE;
if (oldProxy!=null) {
if (mode.wasNotLoaded()) {
- throw new IllegalStateException("call to manage entity "+e+" from unloaded state ("+mode+") but already had proxy "+oldProxy+" already known under that id at "+this);
+ throw new IdAlreadyExistsException("call to manage entity "+e+" from unloaded "
--- End diff --
As above the exception text isnt returned so hard to recover from from
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130405571
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java ---
@@ -110,15 +112,32 @@ public Response createFromYaml(
required = true)
String yaml);
+ @Beta
+ @PUT
+ @Path("/{application}")
+ @Consumes({"application/x-yaml",
+ // see http://stackoverflow.com/questions/332129/yaml-mime-type
+ "text/yaml", "text/x-yaml", "application/yaml"})
+ @ApiOperation(
+ value = "[BETA] Create and start a new application from YAML, with the given id",
+ response = org.apache.brooklyn.rest.domain.TaskSummary.class
+ )
+ @ApiResponses(value = {
+ @ApiResponse(code = 404, message = "Undefined entity or location"),
+ @ApiResponse(code = 409, message = "Application already registered")
--- End diff --
409 is not returned when there is an id clash
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
I believe they use StringShortener to the maximum length required by the cloud see [here](https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/location/cloud/names/BasicCloudMachineNamer.java) @neykov
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130148929
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -101,6 +105,14 @@
@Context
private UriInfo uriInfo;
+ /**
+ * Only lower-case letters and digits; min 10 chars; max 1024 chars.
+ *
+ * We are this extreme because some existing entity implementations rely on the entity-id format
+ * for use in hostnames, etc.
+ */
+ private final Pattern appIdPattern = Pattern.compile("[a-z0-9]{10,1022}");
--- End diff --
For max length, I don't know what a good number is! If we're worried about it being used in cloud resource names, we might need to restrict it to < 64 chars. but that seems far too restrictive for ids (coming back to Mark's point about the use of proper UUID generators).
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
retest this please
---
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] brooklyn-server pull request #782: Adds `PUT /applications/{appid}`
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/782#discussion_r130056079
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -101,6 +105,14 @@
@Context
private UriInfo uriInfo;
+ /**
+ * Only lower-case letters and digits; min 10 chars; max 1024 chars.
+ *
+ * We are this extreme because some existing entity implementations rely on the entity-id format
+ * for use in hostnames, etc.
+ */
+ private final Pattern appIdPattern = Pattern.compile("[a-z0-9]{10,1022}");
--- End diff --
This validation should probably also be deeper ... somewhere in mgmt context
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
Just a heads up that some downstream apps/entities embed the appId in the name of cloud resources and having an ID with a length of 1022 will lead to failures when trying to create them.
Longer term we shouldn't be relying on the IDs for naming obviously, but it's a big troubleshooting help so it's hard to resist :).
---
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] brooklyn-server issue #782: Adds `PUT /applications/{appid}`
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/782
That's used for machine names which already include a bunch of other unique tags, including a jclouds generated random number. Of course we could use a similar strategy, it's just something that's not in place atm.
+1 for the change in general, maybe start with shorter ID restrictions at first, until projects had the time to catch with the changes.
---
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.
---