[GitHub] brooklyn-server issue #300: Location DSL Updates
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/300 @grkvlt the nine test failures in jenkins look related - e.g. `org.apache.brooklyn.entity.group.DynamicFabricTest.testDynamicFabricCreatesAndStartsEntityWhenGivenManyLocations`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r75291309
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/LocationSpecConfiguration.java
---
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.camp.brooklyn.spi.creation;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.Maps;
+
+import org.apache.brooklyn.api.location.LocationSpec;
+
+/**
+ * Captures the {@link LocationSpec} configuration defined in YAML.
+ *
+ * This class does not parse that output; it just stores it.
+ */
+public class LocationSpecConfiguration {
+
+@SuppressWarnings("unused")
+private static final Logger LOG =
LoggerFactory.getLogger(LocationSpecConfiguration.class);
+
+private Map specConfiguration;
+
+public LocationSpecConfiguration(Map specConfiguration) {
+this.specConfiguration =
Maps.newHashMap(checkNotNull(specConfiguration, "specConfiguration"));
--- End diff --
Personal preference for `newLinkedHashMap` (so that logging/display order
is consistent) with what the user supplies.
Seems strange to take a copy here, and to assert non-null; but then in
`setSpecConfiguration` we don't copy it. And if subsequently using the setter,
then will it really pass non-null to the constructor?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r75291679
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
---
@@ -84,51 +89,72 @@ public DslComponent(DslComponent scopeComponent, Scope
scope, String componentId
}
// ---
-
+
@Override
-public Task newTask() {
-return
TaskBuilder.builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-.body(new EntityInScopeFinder(scopeComponent, scope,
componentId)).build();
+public Task newTask() {
+return TaskBuilder.builder()
+.displayName(toString())
+.tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+.body(new ObjectInScopeFinder(scopeComponent, scope,
componentId))
+.build();
}
-
-protected static class EntityInScopeFinder implements Callable
{
+
+protected static class ObjectInScopeFinder implements Callable {
protected final DslComponent scopeComponent;
protected final Scope scope;
protected final String componentId;
-public EntityInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
+public ObjectInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
this.scopeComponent = scopeComponent;
this.scope = scope;
this.componentId = componentId;
}
protected EntityInternal getEntity() {
if (scopeComponent!=null) {
-return (EntityInternal)scopeComponent.get();
+return (EntityInternal) scopeComponent.get();
--- End diff --
Ah, ignore me - I missed the method name!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r75291636
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
---
@@ -84,51 +89,72 @@ public DslComponent(DslComponent scopeComponent, Scope
scope, String componentId
}
// ---
-
+
@Override
-public Task newTask() {
-return
TaskBuilder.builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-.body(new EntityInScopeFinder(scopeComponent, scope,
componentId)).build();
+public Task newTask() {
+return TaskBuilder.builder()
+.displayName(toString())
+.tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+.body(new ObjectInScopeFinder(scopeComponent, scope,
componentId))
+.build();
}
-
-protected static class EntityInScopeFinder implements Callable
{
+
+protected static class ObjectInScopeFinder implements Callable {
protected final DslComponent scopeComponent;
protected final Scope scope;
protected final String componentId;
-public EntityInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
+public ObjectInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
this.scopeComponent = scopeComponent;
this.scope = scope;
this.componentId = componentId;
}
protected EntityInternal getEntity() {
if (scopeComponent!=null) {
-return (EntityInternal)scopeComponent.get();
+return (EntityInternal) scopeComponent.get();
--- End diff --
Seems strange to cast this to `EntityInternal`, now that you've generalised
it to `ObjectInScopeFinder`.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r75292142
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
---
@@ -84,51 +89,72 @@ public DslComponent(DslComponent scopeComponent, Scope
scope, String componentId
}
// ---
-
+
@Override
-public Task newTask() {
-return
TaskBuilder.builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-.body(new EntityInScopeFinder(scopeComponent, scope,
componentId)).build();
+public Task newTask() {
+return TaskBuilder.builder()
+.displayName(toString())
+.tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+.body(new ObjectInScopeFinder(scopeComponent, scope,
componentId))
+.build();
}
-
-protected static class EntityInScopeFinder implements Callable
{
+
+protected static class ObjectInScopeFinder implements Callable {
protected final DslComponent scopeComponent;
protected final Scope scope;
protected final String componentId;
-public EntityInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
+public ObjectInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
this.scopeComponent = scopeComponent;
this.scope = scope;
this.componentId = componentId;
}
protected EntityInternal getEntity() {
if (scopeComponent!=null) {
-return (EntityInternal)scopeComponent.get();
+return (EntityInternal) scopeComponent.get();
} else {
return entity();
}
}
-
+
+protected LocationInternal getLocation() {
+if (scopeComponent!=null) {
+return (LocationInternal) scopeComponent.get();
+} else {
+throw new IllegalStateException("Scope component must be
set");
+}
+}
+
@Override
-public Entity call() throws Exception {
-Iterable entitiesToSearch = null;
+public O call() throws Exception {
EntityInternal entity = getEntity();
--- End diff --
I don't follow what the expectation of this class is now. Do we expect the
`scopeComponent` to always be an entity? Or sometimes a location? If it calls
`call()` then it will fail for anything but an entity, I presume.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r75292379
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
---
@@ -145,81 +171,88 @@ public Entity call() throws Exception {
default:
throw new IllegalStateException("Unexpected scope
"+scope);
}
-
+
Optional result = Iterables.tryFind(entitiesToSearch,
EntityPredicates.configEqualTo(BrooklynCampConstants.PLAN_ID, componentId));
-
+
if (result.isPresent())
-return result.get();
-
+return (O) result.get();
+
// TODO may want to block and repeat on new entities joining?
throw new NoSuchElementException("No entity matching id " +
componentId+
(scope==Scope.GLOBAL ? "" : ", in scope "+scope+" wrt
"+entity+
(scopeComponent!=null ? " ("+scopeComponent+" from
"+entity()+")" : "")));
-}
+}
}
-
+
// ---
// DSL words which move to a new component
-
-public DslComponent entity(String scopeOrId) {
+
+public DslComponent entity(String scopeOrId) {
return new DslComponent(this, Scope.GLOBAL, scopeOrId);
}
-public DslComponent child(String scopeOrId) {
+public DslComponent child(String scopeOrId) {
return new DslComponent(this, Scope.CHILD, scopeOrId);
}
-public DslComponent sibling(String scopeOrId) {
+public DslComponent sibling(String scopeOrId) {
return new DslComponent(this, Scope.SIBLING, scopeOrId);
}
-public DslComponent descendant(String scopeOrId) {
+public DslComponent descendant(String scopeOrId) {
return new DslComponent(this, Scope.DESCENDANT, scopeOrId);
}
-public DslComponent ancestor(String scopeOrId) {
+public DslComponent ancestor(String scopeOrId) {
return new DslComponent(this, Scope.ANCESTOR, scopeOrId);
}
-public DslComponent root() {
+public DslComponent root() {
return new DslComponent(this, Scope.ROOT, "");
}
-public DslComponent scopeRoot() {
+public DslComponent scopeRoot() {
return new DslComponent(this, Scope.SCOPE_ROOT, "");
}
-
+
@Deprecated /** @deprecated since 0.7.0 */
-public DslComponent component(String scopeOrId) {
+public DslComponent component(String scopeOrId) {
return new DslComponent(this, Scope.GLOBAL, scopeOrId);
}
-
-public DslComponent self() {
+
+public DslComponent self() {
return new DslComponent(this, Scope.THIS, null);
}
-
-public DslComponent parent() {
+
+public DslComponent parent() {
return new DslComponent(this, Scope.PARENT, "");
}
-
-public DslComponent component(String scope, String id) {
+
+public DslComponent component(String scope, String id) {
if (!DslComponent.Scope.isValid(scope)) {
throw new IllegalArgumentException(scope + " is not a vlaid
scope");
}
return new DslComponent(this,
DslComponent.Scope.fromString(scope), id);
}
+public DslComponent location() {
+return new DslComponent(this, Scope.LOCATION, null);
+}
+
// DSL words which return things
public BrooklynDslDeferredSupplier entityId() {
-return new EntityId(this);
+return new Identity(this);
}
-protected static class EntityId extends
BrooklynDslDeferredSupplier {
--- End diff --
Is this rename going to be backwards compatible? Could `EntityId` be in
anyone's persisted state?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #300: Location DSL Updates
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/300 No tests have been added/updated - e.g. for the DSL to lookup locations, and for testing what you're adding to `DynamicFabric`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r75293363
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
---
@@ -84,51 +89,72 @@ public DslComponent(DslComponent scopeComponent, Scope
scope, String componentId
}
// ---
-
+
@Override
-public Task newTask() {
-return
TaskBuilder.builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-.body(new EntityInScopeFinder(scopeComponent, scope,
componentId)).build();
+public Task newTask() {
+return TaskBuilder.builder()
+.displayName(toString())
+.tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+.body(new ObjectInScopeFinder(scopeComponent, scope,
componentId))
+.build();
}
-
-protected static class EntityInScopeFinder implements Callable
{
+
+protected static class ObjectInScopeFinder implements Callable {
protected final DslComponent scopeComponent;
protected final Scope scope;
protected final String componentId;
-public EntityInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
+public ObjectInScopeFinder(DslComponent scopeComponent, Scope
scope, String componentId) {
this.scopeComponent = scopeComponent;
this.scope = scope;
this.componentId = componentId;
}
protected EntityInternal getEntity() {
if (scopeComponent!=null) {
-return (EntityInternal)scopeComponent.get();
+return (EntityInternal) scopeComponent.get();
} else {
return entity();
}
}
-
+
+protected LocationInternal getLocation() {
+if (scopeComponent!=null) {
+return (LocationInternal) scopeComponent.get();
+} else {
+throw new IllegalStateException("Scope component must be
set");
+}
+}
+
@Override
-public Entity call() throws Exception {
-Iterable entitiesToSearch = null;
+public O call() throws Exception {
EntityInternal entity = getEntity();
+
+if (scope == Scope.LOCATION) {
+Maybe machine =
Machines.findUniqueMachineLocation(entity.getLocations(),
SshMachineLocation.class);
--- End diff --
Does this mean it will fail if used for a non-machine location? e.g. if
called on the top-level app that has a `JcloudsLocation` instance?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #300: Location DSL Updates
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/300 @grkvlt finished reviewing - a few minor comments; the biggest is about the failing tests, and lack of new tests to cover the new functionality. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-server issue #299: fix extra ssh key data
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/299 The jenkins build failure is unrelated (e.g. `Could not start Jetty server on port 9,998: Address already in use`). LGTM; merging. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-server pull request #299: fix extra ssh key data
Github user asfgit closed the pull request at: https://github.com/apache/brooklyn-server/pull/299 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-docs issue #100: Updated the maven archetype doc
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-docs/pull/100 LGTM; merging. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-docs pull request #100: Updated the maven archetype doc
Github user asfgit closed the pull request at: https://github.com/apache/brooklyn-docs/pull/100 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-library issue #61: Added rubyrep icon
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-library/pull/61 LGTM; merging. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] brooklyn-library pull request #61: Added rubyrep icon
Github user asfgit closed the pull request at: https://github.com/apache/brooklyn-library/pull/61 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
