You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by grkvlt <gi...@git.apache.org> on 2016/08/13 14:34:31 UTC
[GitHub] brooklyn-server pull request #300: Location DSL Updates
GitHub user grkvlt opened a pull request:
https://github.com/apache/brooklyn-server/pull/300
Location DSL Updates
- Adds a `$brooklyn:location()` command to the DSL to enable retrieval of location config keys
- Adds a `$brooklyn:locationSpec:` command to the DSL to enable defining `LocationSpec<?>` objects in YAML
- Updates `DynamicFabric` with configuration key that accepts a list of `LocationSpec<?>` objects that will be used as extra locations to create clusters
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/brooklyn-server location-dsl
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/300.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 #300
----
commit ca2d82eea902e22c162a19f131793996b8c08b86
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-08-12T19:11:59Z
Location YAML DSL additions
commit 31a10941873453622185b95f3e34eca6697c5160
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-08-13T13:39:36Z
Add locationSpec DSL function
commit b8c81574a0aa9e8f34c64c5126b822070507d6dd
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-08-13T13:40:04Z
Add configuration for LocationSpec on DynamicFabric entity
----
---
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 #300: Location DSL Updates
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/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<Entity> newTask() {
- return TaskBuilder.<Entity>builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
- .body(new EntityInScopeFinder(scopeComponent, scope, componentId)).build();
+ public Task<O> newTask() {
+ return TaskBuilder.<O>builder()
+ .displayName(toString())
+ .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+ .body(new ObjectInScopeFinder(scopeComponent, scope, componentId))
+ .build();
}
-
- protected static class EntityInScopeFinder implements Callable<Entity> {
+
+ protected static class ObjectInScopeFinder<O> implements Callable<O> {
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
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/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<Entity> newTask() {
- return TaskBuilder.<Entity>builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
- .body(new EntityInScopeFinder(scopeComponent, scope, componentId)).build();
+ public Task<O> newTask() {
+ return TaskBuilder.<O>builder()
+ .displayName(toString())
+ .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+ .body(new ObjectInScopeFinder(scopeComponent, scope, componentId))
+ .build();
}
-
- protected static class EntityInScopeFinder implements Callable<Entity> {
+
+ protected static class ObjectInScopeFinder<O> implements Callable<O> {
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #300: Location DSL Updates
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/300
@neykov yes referencing the `MachineLocation` is probably what people want right now but conceptually it is a hack to hard code that. i think we should **not** hack things into the `$` DSL: they will be hard to undo.
i'd prefer to look at building out the DSL around *relations* and use that. eg a jquery-ish `$brooklyn:relation("RUNNING_ON_MACHINE_LOCATION").wait("30s").requireUnique().config("host.address")`.
in the interim what about the simple approach of a policy that publishes the `MachineLocation` as a sensor and the DSL allowing `attributeWhenReady("my.machine").config("spec.final")` ?
---
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 #300: Location DSL Updates
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/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<Entity> newTask() {
- return TaskBuilder.<Entity>builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
- .body(new EntityInScopeFinder(scopeComponent, scope, componentId)).build();
+ public Task<O> newTask() {
+ return TaskBuilder.<O>builder()
+ .displayName(toString())
+ .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+ .body(new ObjectInScopeFinder(scopeComponent, scope, componentId))
+ .build();
}
-
- protected static class EntityInScopeFinder implements Callable<Entity> {
+
+ protected static class ObjectInScopeFinder<O> implements Callable<O> {
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<Entity> entitiesToSearch = null;
+ public O call() throws Exception {
EntityInternal entity = getEntity();
+
+ if (scope == Scope.LOCATION) {
+ Maybe<SshMachineLocation> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #300: Location DSL Updates
Posted by aledsage <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt closed the pull request at:
https://github.com/apache/brooklyn-server/pull/300
---
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 #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r79842886
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/DynamicFabric.java ---
@@ -18,39 +18,41 @@
*/
package org.apache.brooklyn.entity.group;
+import java.util.List;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.reflect.TypeToken;
+
import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.entity.ImplementedBy;
+import org.apache.brooklyn.api.location.LocationSpec;
import org.apache.brooklyn.api.sensor.AttributeSensor;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.config.MapConfigKey;
import org.apache.brooklyn.core.entity.Attributes;
-import org.apache.brooklyn.core.entity.factory.EntityFactory;
import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
import org.apache.brooklyn.core.entity.trait.Startable;
import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.util.core.flags.SetFromFlag;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.reflect.TypeToken;
-
/**
- * When a dynamic fabric is started, it starts an entity in each of its locations.
- * This entity will be the parent of each of the started entities.
+ * When a dynamic fabric is started, it starts an entity in each of its locations.
+ * This entity will be the parent of each of the started entities.
*/
@ImplementedBy(DynamicFabricImpl.class)
@SuppressWarnings("serial")
public interface DynamicFabric extends AbstractGroup, Startable, Fabric {
public static final AttributeSensor<Integer> FABRIC_SIZE = Sensors.newIntegerSensor("fabric.size", "Fabric size");
-
+
@SetFromFlag("memberSpec")
public static final ConfigKey<EntitySpec<?>> MEMBER_SPEC = ConfigKeys.newConfigKey(
- new TypeToken<EntitySpec<?>>() {}, "dynamiccfabric.memberspec", "entity spec for creating new cluster members", null);
+ new TypeToken<EntitySpec<?>>() {}, "dynamiccfabric.memberspec", "Entity specification for creating new cluster members", null);
- @SetFromFlag("factory")
- public static final ConfigKey<EntityFactory<?>> FACTORY = ConfigKeys.newConfigKey(
- new TypeToken<EntityFactory<?>>() {}, "dynamicfabric.factory", "factory for creating new cluster members", null);
+ @SetFromFlag("locationSpecs")
+ public static final ConfigKey<List<LocationSpec<?>>> LOCATION_SPECS = ConfigKeys.newConfigKey(
+ new TypeToken<List<LocationSpec<?>>>() {}, "dynamiccfabric.locationspecs", "List of location specifications for starting new cluster members", null);
--- End diff --
Persistence/backwards compatibility concerns.
---
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 #300: Location DSL Updates
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/300
is there an ambiguity eg at a `SoftwareProcess` if i do `$brooklyn:location()` do i get the ssh machine location or the cloud location ... and similar eg at a `Fabric` if there are multiple locations.
note the config keys in `LocationInternal` eg `spec.final` which would be quite useful to retrieve the location spec
---
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 #300: Location DSL Updates
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/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<Entity> 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> entity(String scopeOrId) {
return new DslComponent(this, Scope.GLOBAL, scopeOrId);
}
- public DslComponent child(String scopeOrId) {
+ public DslComponent<Entity> child(String scopeOrId) {
return new DslComponent(this, Scope.CHILD, scopeOrId);
}
- public DslComponent sibling(String scopeOrId) {
+ public DslComponent<Entity> sibling(String scopeOrId) {
return new DslComponent(this, Scope.SIBLING, scopeOrId);
}
- public DslComponent descendant(String scopeOrId) {
+ public DslComponent<Entity> descendant(String scopeOrId) {
return new DslComponent(this, Scope.DESCENDANT, scopeOrId);
}
- public DslComponent ancestor(String scopeOrId) {
+ public DslComponent<Entity> ancestor(String scopeOrId) {
return new DslComponent(this, Scope.ANCESTOR, scopeOrId);
}
- public DslComponent root() {
+ public DslComponent<Entity> root() {
return new DslComponent(this, Scope.ROOT, "");
}
- public DslComponent scopeRoot() {
+ public DslComponent<Entity> scopeRoot() {
return new DslComponent(this, Scope.SCOPE_ROOT, "");
}
-
+
@Deprecated /** @deprecated since 0.7.0 */
- public DslComponent component(String scopeOrId) {
+ public DslComponent<Entity> component(String scopeOrId) {
return new DslComponent(this, Scope.GLOBAL, scopeOrId);
}
-
- public DslComponent self() {
+
+ public DslComponent<Entity> self() {
return new DslComponent(this, Scope.THIS, null);
}
-
- public DslComponent parent() {
+
+ public DslComponent<Entity> parent() {
return new DslComponent(this, Scope.PARENT, "");
}
-
- public DslComponent component(String scope, String id) {
+
+ public DslComponent<Entity> 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> 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<Object> {
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r79832991
--- 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<Entity> newTask() {
- return TaskBuilder.<Entity>builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
- .body(new EntityInScopeFinder(scopeComponent, scope, componentId)).build();
+ public Task<O> newTask() {
+ return TaskBuilder.<O>builder()
+ .displayName(toString())
+ .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+ .body(new ObjectInScopeFinder(scopeComponent, scope, componentId))
+ .build();
}
-
- protected static class EntityInScopeFinder implements Callable<Entity> {
+
+ protected static class ObjectInScopeFinder<O> implements Callable<O> {
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<Entity> entitiesToSearch = null;
+ public O call() throws Exception {
EntityInternal entity = getEntity();
+
+ if (scope == Scope.LOCATION) {
+ Maybe<SshMachineLocation> machine = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class);
--- End diff --
It makes sense to expose only machine locations - our hierarchy of parent "spec" locations is confusing to users (well me). Better hide them altogether - it's the machine locations that are interesting. This has the nice effect for spinning up machines that the `$brooklyn:location().locationId()` will return only when the VM is provisioned. Instead of returning immediately with the ID of the spec location.
Can be improved by:
* working with `MachineLocation` to accomodate the winrm case
* looking at parent entities for the location if there's non at this level - for the case of nested softwareprocesses/sameserverentity.
---
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 #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/300
Changes look good (apart from Aled's comments). Very useful!
I prefer if we don't re-use DslComponent for implementing both entity and location behaviour. The parser supports object chaining so it could be implemented similar to `attributeWhenReady` but returning a `DslLocation` instead.
---
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 #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r79842987
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/DynamicFabric.java ---
@@ -67,9 +69,7 @@
public static final AttributeSensor<Lifecycle> SERVICE_STATE_ACTUAL = Attributes.SERVICE_STATE_ACTUAL;
public void setMemberSpec(EntitySpec<?> memberSpec);
-
- public void setFactory(EntityFactory<?> factory);
-
--- End diff --
Should we go through a deprecation cycle 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.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
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/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<String, Object> specConfiguration;
+
+ public LocationSpecConfiguration(Map<String, ?> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/300
Makes sense @ahgittin.
---
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 #300: Location DSL Updates
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/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<Entity> newTask() {
- return TaskBuilder.<Entity>builder().displayName(toString()).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
- .body(new EntityInScopeFinder(scopeComponent, scope, componentId)).build();
+ public Task<O> newTask() {
+ return TaskBuilder.<O>builder()
+ .displayName(toString())
+ .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+ .body(new ObjectInScopeFinder(scopeComponent, scope, componentId))
+ .build();
}
-
- protected static class EntityInScopeFinder implements Callable<Entity> {
+
+ protected static class ObjectInScopeFinder<O> implements Callable<O> {
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<Entity> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r79833570
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/DynamicFabricImpl.java ---
@@ -134,29 +125,39 @@ public void start(Collection<? extends Location> locsO) {
it = locationsToStart.get(locIndex++ % locationsToStart.size());
((EntityInternal)child).addLocations(Arrays.asList(it));
}
-
+
tasks.put(child, Entities.submit(this,
- Effectors.invocation(child, START, ImmutableMap.of("locations",
+ Effectors.invocation(child, START, ImmutableMap.of("locations",
it==null ? ImmutableList.of() : ImmutableList.of(it))).asTask()));
}
}
// remove all the locations we applied to existing nodes
while (locIndex-->0 && !locationsToStart.isEmpty())
locationsToStart.remove(0);
- // finally (and usually) we create new entities for locations passed in
+ // now look at the configured location specs
+ List<LocationSpec<?>> locationSpecs = MutableList.copyOf(config().get(LOCATION_SPECS));
+ if (locationSpecs.size() > 0) {
+ // use these in addition to locations passed to start method
+ for (LocationSpec<?> spec : locationSpecs) {
+ Location location = getManagementContext().getLocationManager().createLocation(spec);
+ locationsToStart.add(location);
+ }
--- End diff --
Where are these locations unmanaged?
---
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 #300: Location DSL Updates
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/300
I am going to close this, as the `DslComponent` code has moved too much to merge, and I think @neykov's idea of a `DslLocation` might bea better way of handling this concept. Will revisit later.
---
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 #300: Location DSL Updates
Posted by aledsage <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #300: Location DSL Updates
Posted by aledsage <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/300#discussion_r79810923
--- 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<String, Object> specConfiguration;
+
+ public LocationSpecConfiguration(Map<String, ?> specConfiguration) {
+ this.specConfiguration = Maps.newHashMap(checkNotNull(specConfiguration, "specConfiguration"));
--- End diff --
+1 for ordered map - snameyaml's maps are already ordered.
---
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 #300: Location DSL Updates
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/300
@ahgittin I think `$brooklyn:location()` should be referencing the `MachineLocation`. Blocking until available. Perhaps we could add `.getParent()` to get the "location spec".
---
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.
---