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/05/23 23:00:31 UTC
[GitHub] brooklyn-server pull request: [WIP] Add DSL support for calling ef...
GitHub user grkvlt opened a pull request:
https://github.com/apache/brooklyn-server/pull/155
[WIP] Add DSL support for calling effectors in YAML
Adds a new DSL function `$brooklyn:effector` that takes two parameters, a string with the effector name and an optional map of the named effector arguments. Use in a blueprint wherever you can substitute a `$brooklyn` function, like this:
```YAML
thing:
$brooklyn:entity("other").effector("findInformation")
stuff:
$brooklyn:effector:
- "getData"
- arg1: "value"
arg2: 3.14159d
arg3: $brooklyn:config("key")
```
See #153 for a complementary mechanism to create effectors in YAML directly.
### Issues
It is hard to determine when the effector will be executed, but lazy evaluation can be effected by assigning the result of an `attributeWhenReady()` call to one of the effector arguments. It may be useful to have a version of the function that explicitly waits until some sensor has a specific value. Different predicates could be defined for start conditions, or implicit or explicit latches could be used. It would also be interesting to allow the tasks created when running an effector to be used in places like the `install.command` configuration of a `VanillaSoftwareProcess` in place of the SSH command executing task. This would require a version of the function that returned a `Task<?>` rather than a value directly, perhaps via another new function like `$brooklyn:task()`?
Further work is still required to complete this feature, including more tests and documentation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/brooklyn-server dsl-effector
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/155.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 #155
----
commit 405c69bc65c274f60b7281b420003a71161599a9
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-05-23T22:31:13Z
Add DSL support for calling effectors in YAML
----
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
I've PRed my changes to Andrew's branch but can re-do this to master if preferred, once Andrew's is merged.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@drigodwin @grkvlt Reply requested this morning. I'm going to revert at noon if I hear nothing. Something of this magnitude should have been discussed on list, and definitely should not have been merged while still apparently in a `[WIP]` state (no "probably" about it). I know there was related discussion on the list but don't think it was agreed we facilitate config lookup invoking effectors.
---
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: [WIP] Add DSL support for calling ef...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/brooklyn-server/pull/155#issuecomment-221351149
You said: "It is hard to determine when the effector will be executed". But it will follow the same rules as for other deferred config: when someone attempts to retrieve the config value, then it will be executed. So we can use all the standard `launch.latch` config etc to control this. That seems good enough to me. Are there other specific use-cases you're thinking of?
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@drigodwin I had to pull in https://github.com/grkvlt/brooklyn-server/pull/2 manually but it is there in 70d9aca so could you have a 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 issue #155: [WIP] Add DSL support for calling effectors in Y...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@drigodwin @grkvlt @aledsage @neykov WDYT? ^ could we revert?
---
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: [WIP] Add DSL support for calling ef...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/brooklyn-server/pull/155#issuecomment-221351457
I'm not sure I follow when you say: "This would require a version of the function that returned a Task<?> rather than a value directly, perhaps via another new function like $brooklyn:task()?"
Can you elaborate on that use-case, and why you'd need 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] brooklyn-server issue #155: [WIP] Add DSL support for calling effectors in Y...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
Okay, the semantics that `some-config: $brooklyn:effector("foo")` means a single call to `foo` (rather than a call on each lookup) is reasonable, although non-obvious, but that means the call will happen once, _very_ early in the entity's lifecycle (when validation occurs), so it's really a back-door way to trigger initialization actions.
The ideal way to do this I think would be to get on to the `sequence-effector` idea we've talked about. But in the absence of that a simple and I think more transparent way to do this would be to provide an `EffectorOnStartAndStopEntity` which you could do as follows:
```
- type: EffectorOnStartAndStopEntity
id: key-generator-util-entity
start.effector:
component: $brooklyn:entity("target")
effector-name: make-key
effector-params: [ 1, 2, 3]
publish-result-as-sensor: my.key
```
then wherever you need the "populate config on startup" behaviour you add the above as a child that starts in parallel and you set the desired config to `$brooklyn:entity("key-generator-util-entity").attributeWhenReady("my.key")` .
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
I agree this should probably not have been marked as a WIP @ahgittin . This initially came about through work on the CA entity in clocker, there was no good way to share keys between entities. I have no problem reverting it but I think it is a useful addition which we shouldn't throw away without discussion.
---
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: [WIP] Add DSL support for calling ef...
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/155#discussion_r64441168
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/EffectorsYamlTest.java ---
@@ -0,0 +1,66 @@
+/*
+ * 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;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.entity.EntityAsserts;
+import org.apache.brooklyn.core.entity.trait.Startable;
+import org.apache.brooklyn.util.time.Duration;
+
+@Test
+public class EffectorsYamlTest extends AbstractYamlTest {
+ private static final Logger log = LoggerFactory.getLogger(EffectorsYamlTest.class);
+
+ @Test
+ public void testWithAppEnricher() throws Exception {
--- End diff --
Need to have a test with effector that takes parameters, that has a return value, etc.
Instead of looking for the side-effect of stop, I'd use something like `TestEntity`. You can then call `testEntity.getCallHistory` to confirm that the effector was really invoked.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@geomacy I have merged your changes and rebased to master
---
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: [WIP] Add DSL support for calling ef...
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/155#discussion_r64440692
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java ---
@@ -227,6 +230,53 @@ public String toString() {
}
}
+ public BrooklynDslDeferredSupplier<?> effector(final String effectorName) {
+ return new ExecuteEffector(this, effectorName, ImmutableMap.<String, Object>of());
+ }
+ public BrooklynDslDeferredSupplier<?> effector(final String effectorName, final Map<String, ?> args) {
+ return new ExecuteEffector(this, effectorName, args);
+ }
+ // class simply makes the memento XML files nicer
+ protected static class ExecuteEffector extends BrooklynDslDeferredSupplier<Object> {
+ private static final long serialVersionUID = 1740899524088902383L;
+ private final DslComponent component;
+ private final String effectorName;
+ private final Map<String, ?> args;
+ public ExecuteEffector(DslComponent component, String effectorName, Map<String, ?> args) {
+ this.component = Preconditions.checkNotNull(component);
+ this.effectorName = effectorName;
+ this.args = args;
+ }
+ @SuppressWarnings("unchecked")
+ @Override
+ public Task<Object> newTask() {
+ Entity targetEntity = component.get();
+ Maybe<Effector<?>> targetEffector = targetEntity.getEntityType().getEffectorByName(effectorName);
+ if (targetEffector.isAbsentOrNull()) {
+ throw new IllegalArgumentException("Effector " + effectorName + " not found on entity: " + targetEntity);
+ }
+ return (Task<Object>) Entities.invokeEffector(targetEntity, targetEntity, targetEffector.get(), args);
+ }
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(component, effectorName);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
--- End diff --
Should equals include the `args`? Not sure when equals would actually be used in anger.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
I have pulled this and was doing some tests around it. I noticed that given the test YAML
```
services:
- type: org.apache.brooklyn.core.test.entity.TestEntity
id: entity1
brooklyn.config:
test.confName: $brooklyn:component("entity1").effector("myEffector")
```
and a test that does a getConfig, which should evaluate the DSL and invoke the effector:
```
Assert.assertNull(testEntity.getConfig(TestEntity.CONF_NAME));
```
the expectation is that the call history on the test entity should contain one call to \u201cmyEffector\u201d. In fact it contains two, because the config item is evaluated during validation in construction of the entity, in a task kicked of from stack trace
```
"main@1" prio=5 tid=0x1 nid=NA runnable
java.lang.Thread.State: RUNNABLE
at org.apache.brooklyn.util.core.task.ValueResolver.getMaybeInternal(ValueResolver.java:326)
at org.apache.brooklyn.util.core.task.ValueResolver.getMaybe(ValueResolver.java:257)
at org.apache.brooklyn.util.core.task.ValueResolver.get(ValueResolver.java:250)
at org.apache.brooklyn.core.objs.AbstractConfigurationSupportInternal.getNonBlocking(AbstractConfigurationSupportInternal.java:68)
at org.apache.brooklyn.core.config.ConfigConstraints.validateAll(ConfigConstraints.java:126)
at org.apache.brooklyn.core.config.ConfigConstraints.getViolations(ConfigConstraints.java:115)
at org.apache.brooklyn.core.config.ConfigConstraints.assertValid(ConfigConstraints.java:60)
at org.apache.brooklyn.core.objs.proxy.InternalEntityFactory.validateDescendantConfig(InternalEntityFactory.java:288)
at org.apache.brooklyn.core.objs.proxy.InternalEntityFactory.initEntityAndDescendants(InternalEntityFactory.java:306)
at org.apache.brooklyn.core.objs.proxy.InternalEntityFactory.createEntity(InternalEntityFactory.java:188)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.createEntity(LocalEntityManager.java:145)
at org.apache.brooklyn.camp.brooklyn.AbstractYamlTest.createAndStartApplication(AbstractYamlTest.java:128)
at org.apache.brooklyn.camp.brooklyn.AbstractYamlTest.createAndStartApplication(AbstractYamlTest.java:123)
at org.apache.brooklyn.camp.brooklyn.AbstractYamlTest.createAndStartApplication(AbstractYamlTest.java:115)
at org.apache.brooklyn.camp.brooklyn.EffectorsYamlTest.testWithAppEnricher(EffectorsYamlTest.java:39)
```
I've been looking at updating the ValueResolver to be a bit smarter about this but need to do a bit more testing on it before doing a PR.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
right, so the use case is one entity wanting to get information from another. is there no way this can be accommodated using sensors? my philosophical objection is that we're introducing first-class support for a new class of dependency injection:
* **0 - simplest - static**: `DEP.x` is set to a constant
* **1 - blocking - sensor**: `DEP.x` can wait if a value isn't ready yet, ie it is a promise, `$brooklyn:entity(SRC).attributeWhenReady(...)`, which once resolved is always taken as the value
* **2 - triggering - effector**: every lookup to `DEP.x` invokes a call somewhere eg `$brooklyn:entity(SRC).effector(...)`
a widespread use of 2 scares me and it's worth avoiding this if at all possible. it also means lookups aren't idempotent (which is why the `SideEffecting` marker is introduced here, but it isn't going to work.
could your problem with the CA entity be solved another way, if not with waiting on a sensor, by the config pointing at the _source entity_ rather than the key value itself, and whenever it is accessed there is code which invokes the effector to get the key on that source 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: [WIP] Add DSL support for calling ef...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/brooklyn-server/pull/155#issuecomment-222362194
@aledsage I'll post a message to the Brooklyn dev list that answers your questions more completely. I like the idea for combining the arguments in an `args` map, allowing for expansion, as it fits with some other ideas I have, which I will also elaborate on in my post.
---
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 #155: [WIP] Add DSL support for calling effecto...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/155
---
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: [WIP] Add DSL support for calling ef...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/brooklyn-server/pull/155#issuecomment-221352423
I was thinking we might configure this with:
$brooklyn:entity("other").effector("findInformation"):
args:
arg1: "value"
arg2: 3.14159d
arg3: $brooklyn:config("key")
You could imagine (in the future) supporting more options such as timeout, non-blocking, on-error, etc. Putting the arguments inside "args" gives us better extensibility for the future, I think.
---
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 #155: [WIP] Add DSL support for calling effecto...
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/155#discussion_r73860519
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java ---
@@ -275,6 +285,95 @@ public String toString() {
}
}
+ public BrooklynDslDeferredSupplier<?> effector(final String effectorName) {
+ return new ExecuteEffector(this, effectorName, ImmutableMap.<String, Object>of());
+ }
+ public BrooklynDslDeferredSupplier<?> effector(final String effectorName, final Map<String, ?> args) {
+ return new ExecuteEffector(this, effectorName, args);
+ }
+ public BrooklynDslDeferredSupplier<?> effector(final String effectorName, Object... args) {
+ return new ExecuteEffector(this, effectorName, ImmutableList.copyOf(args));
+ }
+ protected static class ExecuteEffector extends BrooklynDslDeferredSupplier<Object> implements HasSideEffects {
+ private static final long serialVersionUID = 1740899524088902383L;
+ private final DslComponent component;
+ private final String effectorName;
+ private final Map<String, ?> args;
+ private final List<? extends Object> argList;
+ private Task<?> cachedTask;
+ public ExecuteEffector(DslComponent component, String effectorName, Map<String, ?> args) {
+ this.component = Preconditions.checkNotNull(component);
+ this.effectorName = effectorName;
+ this.args = args;
+ this.argList = null;
+ }
+ public ExecuteEffector(DslComponent component, String effectorName, List<? extends Object> args) {
+ this.component = Preconditions.checkNotNull(component);
+ this.effectorName = effectorName;
+ this.argList = args;
+ this.args = null;
+ }
+ @SuppressWarnings("unchecked")
+ @Override
+ public Task<Object> newTask() {
+ Entity targetEntity = component.get();
+ Maybe<Effector<?>> targetEffector = targetEntity.getEntityType().getEffectorByName(effectorName);
+ if (targetEffector.isAbsentOrNull()) {
+ throw new IllegalArgumentException("Effector " + effectorName + " not found on entity: " + targetEntity);
+ }
+ if (cachedTask == null) {
--- End diff --
`cachedTask` is accessed from multiple threads. Should we use some locking to make sure we are creating it only once?
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by aledsage-tmp <gi...@git.apache.org>.
Github user aledsage-tmp commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@grkvlt @geomacy what else needs done before we merge 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] brooklyn-server issue #155: [WIP] Add DSL support for calling effectors in Y...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
Just noting the issues with this that we observed earlier, to do with effector invocations with parameters that are DSL expressions:
At present there is no ability to have multi line effector invocations, which would enable simple writing of effector calls where the parameters were themselves DSL expressions. See disabled test "testEffectorMultiLine" in EffectorsYamlTest.java.
Also when doing such invocations on one line there are problems. The test EffectorsYamlIntegrationTest fails. This appears to be a byproduct of [8bfff1635c](https://github.com/apache/brooklyn-server/commit/8bfff1635c77affa19b9a33522ac1ab2ae6d2c8d) Fix config().getNonBlocking for MapConfigKey. It is cancelling the subtask for the effector call during the validation phase of the blueprint (per attached), before it gets evaluated when it\u2019s meant to be. See attached stack trace.
[threads_report.txt](https://github.com/apache/brooklyn-server/files/354719/threads_report.txt)
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@grkvlt @geomacy can we get Geoff's changes (that are specific to the effector invocation) into this PR, and merge it? Or should we close this PR and have Geoff open a new PR with his changes on top of 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] brooklyn-server issue #155: [WIP] Add DSL support for calling effectors in Y...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
+1 to reverting, but retaining the branch for possible future use after more discussion.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
Ouch -- I don't think this is a good idea. Apologies, I hadn't reviewed it because it was marked `[WIP]` -- in any case we shouldn't be merging WIPs :). My main objections are as follows:
* invoking an effector is often a heavyweight task which we should do sparingly, only exposing in the DSL if there is a compelling use case (has this been presented?), and if so can we achieve it a better way?
* the side-effect marker is designed to avoid that but it is likely not to be used often and so we risk assuming things aren't side-effecting when they are
And there is a minor but aggravating issue that there are other PRs I have being reviewed which clean up immediately semantics and this clobbers them. I think it will be easier to put this on top of those if we decide we need it.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
reverted and pushed, with `FormatStringIntegrationTest` manually restored
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
On balance, I agree with @ahgittin that we should revert this, and first merge https://github.com/apache/brooklyn-server/pull/480
The changes in this PR were suggested on the mailing list as part of @grkvlt's message "[PROPOSAL] Enabling Effective Effectors" on 30/05/2016, but it wasn't properly discussed there.
After reverting this, we need to kick off that discussion on the mailing list again. We need to more clearly describe the use-case that motivated this change. We can then discuss the philosophy behind it.
For clarification, I believe the intent for the CA-server use-case was that the effector call happens *only once*; subsequent queries for the config key gets the value from that initial invocation (or blocks until the effector returns, if another thread is still executing it). But let's discuss that more on the mailing list.
---
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 #155: [WIP] Add DSL support for calling effectors in Y...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/155
@aledsage iirc I think this can be merged in its present state but it would be worth recording a todo item for the points noted in https://github.com/apache/brooklyn-server/pull/155#issuecomment-231414860.
---
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.
---