Build failed in Jenkins: brooklyn-master-build #874

[Apache Jenkins Server]

See 

--
[...truncated 6.87 MB...]
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-launcher-common/0.11.0-SNAPSHOT/brooklyn-launcher-common-0.11.0-SNAPSHOT-tests.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-launcher-common/0.11.0-SNAPSHOT/brooklyn-launcher-common-0.11.0-SNAPSHOT-sources.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-launcher-common/0.11.0-SNAPSHOT/brooklyn-launcher-common-0.11.0-SNAPSHOT-test-sources.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-examples-webapps-parent/0.11.0-SNAPSHOT/brooklyn-examples-webapps-parent-0.11.0-SNAPSHOT.pom
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-example-simple-web-cluster/0.11.0-SNAPSHOT/brooklyn-example-simple-web-cluster-0.11.0-SNAPSHOT.pom
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-example-simple-web-cluster/0.11.0-SNAPSHOT/brooklyn-example-simple-web-cluster-0.11.0-SNAPSHOT.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-example-simple-web-cluster/0.11.0-SNAPSHOT/brooklyn-example-simple-web-cluster-0.11.0-SNAPSHOT-tests.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-example-simple-web-cluster/0.11.0-SNAPSHOT/brooklyn-example-simple-web-cluster-0.11.0-SNAPSHOT-sources.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-example-simple-web-cluster/0.11.0-SNAPSHOT/brooklyn-example-simple-web-cluster-0.11.0-SNAPSHOT-test-sources.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-itest/0.11.0-SNAPSHOT/brooklyn-itest-0.11.0-SNAPSHOT.pom
[JENKINS] Archiving 

 to 
org.apache.brooklyn.example/brooklyn-examples-parent/0.11.0-SNAPSHOT/brooklyn-examples-parent-0.11.0-SNAPSHOT.pom
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT.pom
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT-tests.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT-dist.tar.gz
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT-dist.zip
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT-sources.jar
[JENKINS] Archiving 

 to 
org.apache.brooklyn/brooklyn-dist/0.11.0-SNAPSHOT/brooklyn-dist-0.11.0-SNAPSHOT-test-sources.jar
Compressed 119.89 

Jenkins build is back to stable : brooklyn-master-build #873

[Apache Jenkins Server]

See 

Jenkins build is unstable: brooklyn-master-build #872

[Apache Jenkins Server]

See 

[GitHub] brooklyn-server issue #560: Attempted non-determinate test fixes

[ahgittin]

Github user ahgittin commented on the issue:

https://github.com/apache/brooklyn-server/pull/560
  
@aledsage I didn't pull in #485 for fun.  The simplest fix for some of the 
failures involves building an intermediate bundle thus

> Builds on #485 (using an intermediate JAR) for simplicity.

I'll split out the REST portion of #485 and look at your comments after 
which this will be simpler to review.


---
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 #560: Attempted non-determinate test fixes

[aledsage]

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/560
  
Wow, I wasn't expecting to see this set of commits of file-edits based on 
the name of the PR and the description!

It would be great to review + merge your improvements for non-deterministic 
test failures independent of trying to review + merge the `BundleMaker` 
changes. We can then get it merged quickly, and get the benefits immediately.

Can you separate those out into their own PR, rather than building on the 
`BundleMaker` PR (and thus blocking the merging of those changes until that 
other PR is dealt with)?


---
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 #574: don't NPE fail is application is GC'd at ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/brooklyn-server/pull/574


---
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 #574: don't NPE fail is application is GC'd at same ti...

[aledsage]

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/574
  
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 #571: Better error reporting

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/571#discussion_r103570592
  
--- Diff: 
rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
 ---
@@ -374,7 +378,13 @@ public Response createPoly(byte[] 
inputToAutodetectType) {
 
 
 if (spec != null) {
-return launch(potentialYaml, spec);
+try {
+return launch(potentialYaml, spec);
+} catch (Exception e) {
+Exceptions.propagateIfFatal(e);
+log.warn("Failed REST deployment launching "+spec+": "+e);
+throw WebResourceUtils.badRequest(e, "Error launching 
blueprint (autodetecting)");
--- End diff --

How can we tell this is a `badRequest`, rather than some other internal 
error?


---
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 #485: `BundleMaker` utility routines making it easy to...

[aledsage]

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/485
  
@ahgittin (cc @neykov @geomacy) see 
https://github.com/apache/brooklyn-server/pull/576, for the part of this PR 
that adds the `BundleMaker` (without the REST api changes). What do you think 
about merging that now?


---
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 #576: BundleMaker (part1)

[aledsage]

GitHub user aledsage opened a pull request:

https://github.com/apache/brooklyn-server/pull/576

BundleMaker (part1)

This is extracted from https://github.com/apache/brooklyn-server/pull/485 - 
it excludes the changes to the REST api; it improves test coverage and fixes a 
couple of things in `BundleMaker` (generics, and NPE in 
`createJarFromClasspathDir` when there is no manifest).

The intent is that we can merge this soon, without having to wait for 
consensus on the REST api changes.

The first commit is a squashing-together of @ahgittin's  two commits in 
https://github.com/apache/brooklyn-server/pull/485, but with the rest-api 
changes removed. I made no other changes to that commit (hence the author being 
Alex).

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/aledsage/brooklyn-server 
install-osgi-bundles-part1

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/576.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 #576


commit 33f7940c18ba8cd17fd83500486e02376d3b8fd8
Author: Alex Heneveld 
Date:   2016-12-12T15:43:50Z

`BundleMaker` utility routines making it easy to add osgi bundles 
dynamically

and testing things like inserting a manifest and loading files from the 
bundles

commit 7d1d47b6e8c3ef919d1d52be3d23a5dc036a2d5c
Author: Aled Sage 
Date:   2017-02-28T22:04:35Z

BundleMaker: add tests, and fix

commit f20cdecd2fef5c7ff2f6ef33ab7731c7acfb6de4
Author: Aled Sage 
Date:   2017-02-28T22:26:55Z

CatalogBomScanner tidy




---
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 #318: Delete most deprecated methods from PortF...

[sjcorbett]

Github user sjcorbett closed the pull request at:

https://github.com/apache/brooklyn-server/pull/318


---
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 #485: `BundleMaker` utility routines making it easy to...

[aledsage]

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/485
  
@ahgittin agree there is 100% support on the direction.

There was push-back on what exactly the REST API piece should look like:
* @geomacy suggested that we should only support a zip that had a valid 
MANIFEST.MF file, and that the `br` command could auto-generate that manifest 
to make the user's life simpler.
* @neykov asked why we should require the bundle symbolic name and version 
being passed at all, suggesting they could be inferred from the the catalog.bom 
in some way (most likely with additional metadata).

It also sounded from the mailing list thread like enabling 
`BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM` could be 
fiddly/hard (to make it work in HA mode, on rebind, etc).

It would be good to separate out the two (or three?) things that this PR 
does so that we can merge them incrementally, rather than them all being 
blocked until there's consensus on everything:

1. The underlying building blocks for manipulating bundles etc could get 
merged asap.
2. The changes to the REST api seem like they need more discussion on the 
mailing list.

We should also discuss on the mailing list more about how 
`FEATURE_LOAD_BUNDLE_CATALOG_BOM` should really work - e.g. how will we solve 
the HA/rebind issues. And we should write down (on the mailing list) what 
exactly will be supported for such bundles - presumably "just" reading a root 
`catalog.bom` file; this would reference other things in the bundle using 
`classpath://...` which is an ugly java'ism, but a good starting point.


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on the issue:

https://github.com/apache/brooklyn-server/pull/480
  
@aledsage I think I've addressed these comments, TY; but now merge 
conflicts with #155 are hurting; as noted there I think we should revert that.


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103505398
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -512,6 +559,24 @@ public Object call() throws Exception {
 }
 }
 
+protected Maybe execImmediate(ExecutionContext exec, Object 
immediateSupplierOrImmediateTask) {
+Maybe result;
+try {
+result = exec.getImmediately(immediateSupplierOrImmediateTask);
+} catch (ImmediateSupplier.ImmediateUnsupportedException e) {
+return null;
--- End diff --

+1


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103504542
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -352,33 +357,73 @@ public T get() {
 
 //if the expected type is a closure or map and that's what we 
have, we're done (or if it's null);
 //but not allowed to return a future or DeferredSupplier as the 
resolved value
-if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v)))
+if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v) && 
!TaskFactory.class.isInstance(v)))
 return Maybe.of((T) v);
 
 try {
-if (immediately && v instanceof ImmediateSupplier) {
-final ImmediateSupplier supplier = 
(ImmediateSupplier) v;
+boolean allowImmediateExecution = false;
+boolean bailOutAfterImmediateExecution = false;
+
+if (v instanceof ImmediateSupplier) {
+allowImmediateExecution = true;
+
+} else {
+if ((v instanceof TaskFactory) && !(v instanceof 
DeferredSupplier)) {
+v = ((TaskFactory)v).newTask();
+allowImmediateExecution = true;
+bailOutAfterImmediateExecution = true;
+
BrooklynTaskTags.setTransient(((TaskAdaptable)v).asTask());
+if (isEvaluatingImmediately()) {
+// not needed if executing immediately
+BrooklynTaskTags.addTagDynamically( 
((TaskAdaptable)v).asTask(), BrooklynTaskTags.IMMEDIATE_TASK_TAG );
+}
+}
+
+//if it's a task or a future, we wait for the task to 
complete
+if (v instanceof TaskAdaptable) {
+v = ((TaskAdaptable) v).asTask();
+}
+}
+
+if (allowImmediateExecution && isEvaluatingImmediately()) {
+// TODO could allow for everything, when evaluating 
immediately -- but if the target isn't safe to run again
+// then we have to fail if immediate didn't work; to avoid 
breaking semantics we only do that for a few cases;
+// might be nice to get to the point where we can break 
those semantics however, 
+// ie weakening what getImmediate supports and making it 
be non-blocking, so that bailOut=true is the default.
+// if: v instanceof TaskFactory -- it is safe, it's a new 
API (but it is currently the only one supported);
+// more than safe, we have to do it -- or add code 
here to cancel tasks -- because it spawns new tasks
+// (other objects passed through here don't get 
cancelled, because other things might try again later;
+// ie a task or future passed in here might naturally 
be long-running so cancelling is wrong,
+// but with a task factory generated task it would 
leak if we submitted and didn't cancel!)
--- End diff --

aha, `DST` which we mostly use is fine, but its `doCancel` method was 
missing from the lighter-weight `CompoundTask`; have added it there and test 
passes


---
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 #558: BROOKLYN-433: regex/obj config key constraint in...

[aledsage]

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/558
  
Docs PR now merged - https://github.com/apache/brooklyn-docs/pull/151


---
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.
---

[jira] [Commented] (BROOKLYN-433) YAML-based config constraint to support regex

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888452#comment-15888452
 ] 

ASF GitHub Bot commented on BROOKLYN-433:
-

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/558
  
Docs PR now merged - https://github.com/apache/brooklyn-docs/pull/151


> YAML-based config constraint to support regex
> -
>
> Key: BROOKLYN-433
> URL: https://issues.apache.org/jira/browse/BROOKLYN-433
> Project: Brooklyn
>  Issue Type: Improvement
>Affects Versions: 0.10.0
>Reporter: Aled Sage
>Assignee: Aled Sage
>Priority: Minor
>
> One can specify constraints on a config key's value, to give a validation 
> error if an invalid value is supplied.
> In the Java code, this can be any predicate.
> However, in YAML we don't support many options. It would be good to support a 
> regex constraint. For example:
> {noformat}
> brooklyn.parameters:
> - name: address
>   type: string
>   constraints:
>   - required
>   - regex: ^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$
> {noformat}
> See docs at 
> https://raw.githubusercontent.com/apache/brooklyn-docs/master/guide/yaml/yaml-reference.md,
>  and code at 
> https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java#L204-L205



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (BROOKLYN-433) YAML-based config constraint to support regex

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888445#comment-15888445
 ] 

ASF GitHub Bot commented on BROOKLYN-433:
-

Github user asfgit closed the pull request at:

https://github.com/apache/brooklyn-docs/pull/151


> YAML-based config constraint to support regex
> -
>
> Key: BROOKLYN-433
> URL: https://issues.apache.org/jira/browse/BROOKLYN-433
> Project: Brooklyn
>  Issue Type: Improvement
>Affects Versions: 0.10.0
>Reporter: Aled Sage
>Assignee: Aled Sage
>Priority: Minor
>
> One can specify constraints on a config key's value, to give a validation 
> error if an invalid value is supplied.
> In the Java code, this can be any predicate.
> However, in YAML we don't support many options. It would be good to support a 
> regex constraint. For example:
> {noformat}
> brooklyn.parameters:
> - name: address
>   type: string
>   constraints:
>   - required
>   - regex: ^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$
> {noformat}
> See docs at 
> https://raw.githubusercontent.com/apache/brooklyn-docs/master/guide/yaml/yaml-reference.md,
>  and code at 
> https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java#L204-L205



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] brooklyn-server issue #562: Provide a scratchpad area per management context

[neykov]

Github user neykov commented on the issue:

https://github.com/apache/brooklyn-server/pull/562
  
Good point about casts, updated.


---
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 #151: BROOKLYN-433: docs for yaml config key cons...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/brooklyn-docs/pull/151


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103500728
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -352,33 +357,73 @@ public T get() {
 
 //if the expected type is a closure or map and that's what we 
have, we're done (or if it's null);
 //but not allowed to return a future or DeferredSupplier as the 
resolved value
-if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v)))
+if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v) && 
!TaskFactory.class.isInstance(v)))
 return Maybe.of((T) v);
 
 try {
-if (immediately && v instanceof ImmediateSupplier) {
-final ImmediateSupplier supplier = 
(ImmediateSupplier) v;
+boolean allowImmediateExecution = false;
+boolean bailOutAfterImmediateExecution = false;
+
+if (v instanceof ImmediateSupplier) {
+allowImmediateExecution = true;
+
+} else {
+if ((v instanceof TaskFactory) && !(v instanceof 
DeferredSupplier)) {
+v = ((TaskFactory)v).newTask();
+allowImmediateExecution = true;
+bailOutAfterImmediateExecution = true;
+
BrooklynTaskTags.setTransient(((TaskAdaptable)v).asTask());
+if (isEvaluatingImmediately()) {
+// not needed if executing immediately
+BrooklynTaskTags.addTagDynamically( 
((TaskAdaptable)v).asTask(), BrooklynTaskTags.IMMEDIATE_TASK_TAG );
+}
+}
+
+//if it's a task or a future, we wait for the task to 
complete
+if (v instanceof TaskAdaptable) {
+v = ((TaskAdaptable) v).asTask();
--- End diff --

as above, definitely not


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103500687
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
 ---
@@ -96,7 +98,50 @@ public ExecutionManager getExecutionManager() {
 /** returns tasks started by this context (or tasks which have all the 
tags on this object) */
 @Override
 public Set getTasks() { return 
executionManager.getTasksWithAllTags(tags); }
- 
+
+/** performs execution without spawning a new task thread, though it 
does temporarily set a fake task for the purpose of getting context;
+ * currently supports suppliers or callables  */
+@SuppressWarnings("unchecked")
+@Override
+public  Maybe getImmediately(Object callableOrSupplier) {
+BasicTask fakeTaskForContext;
+if (callableOrSupplier instanceof BasicTask) {
+fakeTaskForContext = (BasicTask)callableOrSupplier;
+if (fakeTaskForContext.isQueuedOrSubmitted()) {
+if (fakeTaskForContext.isDone()) {
+return Maybe.of((T)fakeTaskForContext.getUnchecked());
+} else {
+throw new ImmediateUnsupportedException("Task is in 
progress and incomplete: "+fakeTaskForContext);
+}
+}
+callableOrSupplier = fakeTaskForContext.getJob();
--- End diff --

as above


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103498263
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
 ---
@@ -96,7 +98,50 @@ public ExecutionManager getExecutionManager() {
 /** returns tasks started by this context (or tasks which have all the 
tags on this object) */
 @Override
 public Set getTasks() { return 
executionManager.getTasksWithAllTags(tags); }
- 
+
+/** performs execution without spawning a new task thread, though it 
does temporarily set a fake task for the purpose of getting context;
+ * currently supports suppliers or callables  */
+@SuppressWarnings("unchecked")
+@Override
+public  Maybe getImmediately(Object callableOrSupplier) {
+BasicTask fakeTaskForContext;
+if (callableOrSupplier instanceof BasicTask) {
+fakeTaskForContext = (BasicTask)callableOrSupplier;
--- End diff --

as noted above leaving the task running in background (or possibly not 
submitting it) is the right thing to do for immediate execution of a task; 
cancelling it is risky unless we know the task is dedicated to this one usage 
(hence preferring `TaskFactory`); in particular 
`EntityConfigTest.testGetTaskNonBlockingKey()` fails if 
`allowImmediateExecution=true`


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103499754
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
 ---
@@ -96,7 +98,50 @@ public ExecutionManager getExecutionManager() {
 /** returns tasks started by this context (or tasks which have all the 
tags on this object) */
 @Override
 public Set getTasks() { return 
executionManager.getTasksWithAllTags(tags); }
- 
+
+/** performs execution without spawning a new task thread, though it 
does temporarily set a fake task for the purpose of getting context;
+ * currently supports suppliers or callables  */
+@SuppressWarnings("unchecked")
+@Override
+public  Maybe getImmediately(Object callableOrSupplier) {
+BasicTask fakeTaskForContext;
+if (callableOrSupplier instanceof BasicTask) {
+fakeTaskForContext = (BasicTask)callableOrSupplier;
--- End diff --

As for the particular code block marked here, it is invoked from 
`ValueResolver` when you supply a `TaskFactory`; `VR` generates the `Task` and 
passes it in with `allowImmediateExecution=true`


---
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 #338: Support nested catalog item definitions - testCa...

[geomacy]

Github user geomacy commented on the issue:

https://github.com/apache/brooklyn-server/pull/338
  
Thanks @ahgittin and @neykov for comments; I will try to address the 
remaining items as noted above 
(https://github.com/apache/brooklyn-server/pull/338#issuecomment-281950519) 
soon.

I will have a look at #573 to see how this can be merged with it.

Just on the point of omitting or retaining the `catalogItemId` member 
(https://github.com/apache/brooklyn-server/pull/338#discussion_r102679601),  
for rebind purposes, what do you think of my changes to 
`BrooklynMementoPersisterToObjectStore` which as far as I can see allow me to 
avoid keeping that member.  I am able to re-run all the tests from 
`testRebindWithCatalogAndApp` with persisted content with the format prior to 
this change (see 
https://github.com/apache/brooklyn-server/pull/338/files#diff-3804f33dfb6d89e123adcff13f9c114dR173).
   It looked to me as if we don't need to retain that member, hence the changes 
above, but if you think there's another case to cover then I can put it back 
(and will probably need to think about adding more tests to catch that case).


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103494117
  
--- Diff: 
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
 ---
@@ -554,15 +562,55 @@ protected String resolveKeyName(boolean immediately) {
 .displayName("retrieving config for "+keyName)
 .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
 .dynamic(false)
-.body(new Callable() {
-@Override
-public Object call() throws Exception {
-Entity targetEntity = component.get();
-String keyNameS = resolveKeyName(true);
-ConfigKey key = 
targetEntity.getEntityType().getConfigKey(keyNameS);
-return targetEntity.getConfig(key != null ? 
key : ConfigKeys.newConfigKey(Object.class, keyNameS));
-}})
-.build();
+.body(newCallable(false)).build();
+}
+
+private Callable newCallable(final boolean immediate) {
--- End diff --

yeah it's ugly; at least it's private.  have renamed.


---
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 #338: Support nested catalog item definitions - testCa...

[ahgittin]

Github user ahgittin commented on the issue:

https://github.com/apache/brooklyn-server/pull/338
  
Very cool.  Haven't reviewed in depth but looks on point at a high level.  
A few specific comments:

* this will conflict with 
https://github.com/apache/brooklyn-server/pull/573 -- should be easy to 
resolve, and this is a better solution to the underlying problem

* I think we want _both_ `catalogItemId` with the semantics of #573 and the 
`stack` introduced here for searching, on the spec (that's what we have on 
entity)

* there may be persistence errors migrating from older version if we 
persist specs and remove the `catalogItemId` here (?); the previous suggestion 
to keep both should resolve this (perhaps with an additional check when 
initializing an entity that if `catalogItemId` is set then `stack.get(0)` 
equals that id

And one general comment -- there is an issue if e.g. `my-cluster` and 
`my-node` are set in the same BOM, both at version `1` with the former 
referring to the latter omitting the version.  If we then install a version `2` 
of that bom, then `my-cluster:1` creates a cluster of `my-node:2` which is 
surprising.  It would be nice to solve that (and also be able to retrieve the 
`bom` file and bundle list where an entity is defined, for subsequent 
editting).  It's a separate problem to this, but the solutions may be 
interrelated.  Very happy to have this merged (once someone else does a 
detailed review and we resolve #573 conflicts) without solving the version and 
catalog-group / bom questions, expecting a bit more change afterwards, but 
wanted to flag that.  cc @geomacy @neykov @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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server issue #563: [WIP] Don't SO_REUSEADDR when probing for availa...

[neykov]

Github user neykov commented on the issue:

https://github.com/apache/brooklyn-server/pull/563
  
Using non-`SO_REUSEADDR` sockets seems to work fine on the apache build 
slaves. On the other hand it prevents us from re-binding to the same port after 
restarting Brooklyn, so not ideal.
If the problems persist I suggest we add troubleshooting info in the tests. 
Things like the output of `netstat`, `ip addr show` & similar, which could 
guide us to the root cause of the failures. See 
https://github.com/apache/incubator-brooklyn/pull/657 for other ideas.

https://github.com/apache/incubator-brooklyn/pull/662 addresses a similar 
problem, worth having in mind if someone tries to dig in deeper.


---
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 #562: Provide a scratchpad area per management ...

[geomacy]

Github user geomacy commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/562#discussion_r103490624
  
--- Diff: 
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/BrooklynCampPlatform.java
 ---
@@ -65,7 +65,7 @@ public BrooklynCampPlatform(PlatformRootSummary root, 
ManagementContext manageme
 
 /** finds and returns the {@link CampPlatform} registered for the 
management context, or null if none */
 @Nullable public static CampPlatform findPlatform(ManagementContext 
mgmt) {
-return 
mgmt.getConfig().getConfig(BrooklynCampConstants.CAMP_PLATFORM);
+return (CampPlatform) 
mgmt.getScratchpad().get(BrooklynCampConstants.CAMP_PLATFORM);
--- End diff --

Don't need the cast, here and elsewhere.


---
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.
---

[jira] [Commented] (BROOKLYN-433) YAML-based config constraint to support regex

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888351#comment-15888351
 ] 

ASF GitHub Bot commented on BROOKLYN-433:
-

Github user nakomis commented on the issue:

https://github.com/apache/brooklyn-docs/pull/151
  
LGTM


> YAML-based config constraint to support regex
> -
>
> Key: BROOKLYN-433
> URL: https://issues.apache.org/jira/browse/BROOKLYN-433
> Project: Brooklyn
>  Issue Type: Improvement
>Affects Versions: 0.10.0
>Reporter: Aled Sage
>Assignee: Aled Sage
>Priority: Minor
>
> One can specify constraints on a config key's value, to give a validation 
> error if an invalid value is supplied.
> In the Java code, this can be any predicate.
> However, in YAML we don't support many options. It would be good to support a 
> regex constraint. For example:
> {noformat}
> brooklyn.parameters:
> - name: address
>   type: string
>   constraints:
>   - required
>   - regex: ^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$
> {noformat}
> See docs at 
> https://raw.githubusercontent.com/apache/brooklyn-docs/master/guide/yaml/yaml-reference.md,
>  and code at 
> https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java#L204-L205



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] brooklyn-docs issue #151: BROOKLYN-433: docs for yaml config key constraints

[nakomis]

Github user nakomis commented on the issue:

https://github.com/apache/brooklyn-docs/pull/151
  
LGTM


---
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 #575: Clean up location metadata and OSGi packa...

[ahgittin]

GitHub user ahgittin opened a pull request:

https://github.com/apache/brooklyn-server/pull/575

Clean up location metadata and OSGi packaging

Discovered when trying to use the REST API that we didn't get much of the 
location detail we might want; in particular we didn't have info on 
provider/endpoint/region, and the parent spec was not accurately identified.  
Tracked the latter to confusion around `ORIGINAL_SPEC` which I've clarified and 
deprecated the confusing other field, and the former to a place where flags 
weren't pulled in, just config.

Also had trouble accessing some of the data because it required a cast to 
`JcloudsLocation`, which pulled in an OSGi dependency on the main 
brooklyn-jclouds package, and classes in that package pulled in jclouds bundles 
through their signatures, giving a huge set of bundle constraints in my 
downstream project.  Have refactored here so that we can access many common 
things through an `api` package which doesn't pull in jclouds dependencies.

Finally, noticed some other places where more machine detail would probably 
be wanted, but wasn't provided.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ahgittin/brooklyn-server locations-more-detail

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/575.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 #575


commit bfbcddd5d2088ce2cdeedc1b70e5db1edf37a0fb
Author: Alex Heneveld 
Date:   2017-02-18T00:50:28Z

provide more detail of locations in REST calls

with some confused parts marked TODO

commit e2319e5e1ebf13551a414182965034440c6b40ff
Author: Alex Heneveld 
Date:   2017-02-28T14:11:08Z

extract many API methods to an api package so osgi projects can access much 
functionality
without needing to pull in an exported jclouds dependency

(because JcloudsLocation has methods which access jclouds objects in the 
signature,
use of the jclouds package _exports_ a bunch of jclouds osgi deps which is 
rarely wanted)

this has been done in a backwards compatible way; the classes in the 
package look the same,
there are just now intermediate interfaces which are jclouds-free

commit 1e8873a8d52a03267cbbbd06af1e70ba9a57fcf3
Author: Alex Heneveld 
Date:   2017-02-28T16:01:05Z

sort out ORIGINAL_SPEC and NAMED_SPEC_NAME, deprecating the latter

and ensuring the former is always set to the originally requested spec, 
with tests




---
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.
---

[jira] [Commented] (BROOKLYN-433) YAML-based config constraint to support regex

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888329#comment-15888329
 ] 

ASF GitHub Bot commented on BROOKLYN-433:
-

GitHub user aledsage opened a pull request:

https://github.com/apache/brooklyn-docs/pull/151

BROOKLYN-433: docs for yaml config key constraints



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/aledsage/brooklyn-docs BROOKLYN-433

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-docs/pull/151.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 #151


commit d4650670dee257e33ca0cf595718ef7a796cde79
Author: Aled Sage 
Date:   2017-02-28T16:01:36Z

BROOKLYN-433: docs for yaml config key constraints




> YAML-based config constraint to support regex
> -
>
> Key: BROOKLYN-433
> URL: https://issues.apache.org/jira/browse/BROOKLYN-433
> Project: Brooklyn
>  Issue Type: Improvement
>Affects Versions: 0.10.0
>Reporter: Aled Sage
>Assignee: Aled Sage
>Priority: Minor
>
> One can specify constraints on a config key's value, to give a validation 
> error if an invalid value is supplied.
> In the Java code, this can be any predicate.
> However, in YAML we don't support many options. It would be good to support a 
> regex constraint. For example:
> {noformat}
> brooklyn.parameters:
> - name: address
>   type: string
>   constraints:
>   - required
>   - regex: ^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$
> {noformat}
> See docs at 
> https://raw.githubusercontent.com/apache/brooklyn-docs/master/guide/yaml/yaml-reference.md,
>  and code at 
> https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java#L204-L205



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (BROOKLYN-445) Search path and meaning of catalogItemId is overloaded

[Svetoslav Neykov (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888330#comment-15888330
 ] 

Svetoslav Neykov commented on BROOKLYN-445:
---

[~alex.heneveld] See https://github.com/apache/brooklyn-server/pull/338 for a 
variant of "searchable catalog item IDs". The idea being that there are 
multiple catalalog items which combine (inherit) to produce the final spec.
It's only part of the solution to correctly loading resources. We should be 
looking at the ancestors (at runtime) to try and load resources.

> Search path and meaning of catalogItemId is overloaded
> --
>
> Key: BROOKLYN-445
> URL: https://issues.apache.org/jira/browse/BROOKLYN-445
> Project: Brooklyn
>  Issue Type: Bug
>Affects Versions: 0.10.0
>Reporter: Alex Heneveld
>
> We use catalogItemId on a spec or BrooklynObject for two things:
> (1) Record the catalog item that was defined to create an item
> (2) Find the search path to use when looking up resources and/or creating 
> other specs
> Most of the time these two are the same, e.g. an entity comes from catalog 
> item `cassandra-node:1.0` and the implementation should use the bundles 
> defined against that to resolve scripts etc.  The catalogItemId is the only 
> record of the catalog-entry that was used to define the entity; the entity 
> spec reduces it to the java type which might tell you one bundle but won't 
> tell you of other library entries from the definition.
> However in some cases we seem to want the context catalogItemId to be 
> inherited, e.g. we reference a stock type like `VanillaServer` as a child but 
> supply config pointing at scripts/images in the parent's bundle.  This is 
> currently achieved by a line in AbstractBrooklynObjectSpec's constructor 
> which sets the catalogItemId from the thread context entity's catalog item 
> ID.  However there are a couple problems with this:
> (A) We can't tell if the `catalogItemId` is the definition of an entity (it 
> usually is, but sometimes might be inherited), so we can't tell what was used 
> to create an entity
> (B) A child's search behaviour differs depending whether that child comes 
> from another catalogItemId (which will overwrite the inherited context 
> catalogItemId) or a stock item (e.g. a java type is passed to the spec 
> constructor)
> (C) When setting the EntityDynamicType we can't tell whether to clear the 
> config keys set there; in InternalEntityFactory.addSpecParameters we need to 
> know whether the spec extends the Java type defining it (in which case CAMP 
> code in BasicSpecParameter.initializeSpecWithExplicitParameters has filtered 
> out those keys which are not type-definition inheritable and set the 
> spec.parameters, so the EntityDynamicType's keys should be cleared) or not 
> (in which case spec.parameters won't normally be set and EDT.configKeys 
> should not be cleared).  Currently it checks whether there is a 
> catalogItemId, and assume it is set iff the spec is extending (former case); 
> this assumption fails if catalogItemId is inherited from context.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] brooklyn-docs pull request #151: BROOKLYN-433: docs for yaml config key cons...

[aledsage]

GitHub user aledsage opened a pull request:

https://github.com/apache/brooklyn-docs/pull/151

BROOKLYN-433: docs for yaml config key constraints



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/aledsage/brooklyn-docs BROOKLYN-433

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-docs/pull/151.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 #151


commit d4650670dee257e33ca0cf595718ef7a796cde79
Author: Aled Sage 
Date:   2017-02-28T16:01:36Z

BROOKLYN-433: docs for yaml config key constraints




---
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 #155: [WIP] Add DSL support for calling effectors in Y...

[ahgittin]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server issue #562: Provide a scratchpad area per management context

[neykov]

Github user neykov commented on the issue:

https://github.com/apache/brooklyn-server/pull/562
  
@geomacy Addressed comments, can you take another look.


---
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 #155: [WIP] Add DSL support for calling effectors in Y...

[drigodwin]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server issue #155: [WIP] Add DSL support for calling effectors in Y...

[ahgittin]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #155: [WIP] Add DSL support for calling effecto...

[asfgit]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (BROOKLYN-444) Effector call works from GUI but fails from CLI

[Geoff Macartney (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888160#comment-15888160
 ] 

Geoff Macartney commented on BROOKLYN-444:
--

I've investigated this and the problem lies in the version of CXF used by the 
REST server.

The request from the CLI is valid, noting that there is no content and so 
no request body (and Content-Length is zero). 

{code}
POST /v1/applications/effector-test/entities/esp/effectors/DoStuff HTTP/1.1
Host: geoffs-macbook-pro.local:8081
User-Agent: Go-http-client/1.1
Content-Length: 0
Authorization: Basic BlaBlaBlaBlaBla=
Content-Type: application/x-www-form-urlencoded
Accept-Encoding: gzip
{code}

The problem is in the 
{{org.apache.cxf.jaxrs.utils.FormUtils.populateMapFromString()}} method, 
which, in the 3.1.4 version we are using, is assuming that the request body 
will not be null,
but will have at least one pair of parameters. This was fixed in commit 
https://github.com/apache/cxf/commit/2653e88603fcc690dad279f362a9bf4daf7706bd
which is present in CXF releases from 3.1.6 onward.

This problem should therefore be resolved when we move up to more recent CXF 
versions,
specifically when https://github.com/apache/brooklyn-server/pull/547 is merged.




> Effector call works from GUI but fails from CLI
> ---
>
> Key: BROOKLYN-444
> URL: https://issues.apache.org/jira/browse/BROOKLYN-444
> Project: Brooklyn
>  Issue Type: Bug
>Affects Versions: 0.10.0
> Environment: Brooklyn Version 0.10.0 on Mac
>Reporter: Murdo Aird
>Assignee: Geoff Macartney
>Priority: Critical
>
> The following simple BP adds an Effector to an entity. The entity can be 
> called from the web UI and succeeds. Drilling into the Activity logs shows 
> the simple bash command works.
> However if I try to invoke the Effector via the CLI [1], the call fails:
> br app "effector-test" entity "Empty Software Process" effector DoStuff invoke
> For the CLI failure, it looks like a random (?) environment variable is being 
> injected via the CLI [2].
> {code:none}
> name: effector-test
> location: localhost
> services:
>   - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
> install.command: echo True
> launch.command: echo True
> checkRunning.command: echo true
> brooklyn.initializers:
>   - type: org.apache.brooklyn.core.effector.ssh.SshCommandEffector
> brooklyn.config:
>   name: DoStuff
>   command: whoami
> {code}
> * Calling a standard Effector (e.g. stop) via the CLI works. 
> * If the DoSuff Effector via CLI fails, I can subsequently call it from the 
> web UI and it works
> Not to muddy the waters, but calling the same Effector from the REST API also 
> results in an error - but it's a different one [3].
> [1] https://gist.github.com/murdoaird/0ba46476b8ec1e5039097b3754f720f7
> [2] https://gist.github.com/murdoaird/50777a5fa398a79611b4ec5bbfdc1d7d#env-1
> [3] https://gist.github.com/murdoaird/e153c5b3502d5475dee7358e2d9c3472



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] brooklyn-server issue #155: [WIP] Add DSL support for calling effectors in Y...

[grkvlt]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103461269
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -352,33 +357,73 @@ public T get() {
 
 //if the expected type is a closure or map and that's what we 
have, we're done (or if it's null);
 //but not allowed to return a future or DeferredSupplier as the 
resolved value
-if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v)))
+if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v) && 
!TaskFactory.class.isInstance(v)))
 return Maybe.of((T) v);
 
 try {
-if (immediately && v instanceof ImmediateSupplier) {
-final ImmediateSupplier supplier = 
(ImmediateSupplier) v;
+boolean allowImmediateExecution = false;
+boolean bailOutAfterImmediateExecution = false;
+
+if (v instanceof ImmediateSupplier) {
+allowImmediateExecution = true;
+
+} else {
+if ((v instanceof TaskFactory) && !(v instanceof 
DeferredSupplier)) {
+v = ((TaskFactory)v).newTask();
+allowImmediateExecution = true;
+bailOutAfterImmediateExecution = true;
+
BrooklynTaskTags.setTransient(((TaskAdaptable)v).asTask());
+if (isEvaluatingImmediately()) {
+// not needed if executing immediately
+BrooklynTaskTags.addTagDynamically( 
((TaskAdaptable)v).asTask(), BrooklynTaskTags.IMMEDIATE_TASK_TAG );
+}
+}
+
+//if it's a task or a future, we wait for the task to 
complete
+if (v instanceof TaskAdaptable) {
+v = ((TaskAdaptable) v).asTask();
+}
+}
+
+if (allowImmediateExecution && isEvaluatingImmediately()) {
+// TODO could allow for everything, when evaluating 
immediately -- but if the target isn't safe to run again
+// then we have to fail if immediate didn't work; to avoid 
breaking semantics we only do that for a few cases;
+// might be nice to get to the point where we can break 
those semantics however, 
+// ie weakening what getImmediate supports and making it 
be non-blocking, so that bailOut=true is the default.
+// if: v instanceof TaskFactory -- it is safe, it's a new 
API (but it is currently the only one supported);
+// more than safe, we have to do it -- or add code 
here to cancel tasks -- because it spawns new tasks
+// (other objects passed through here don't get 
cancelled, because other things might try again later;
+// ie a task or future passed in here might naturally 
be long-running so cancelling is wrong,
+// but with a task factory generated task it would 
leak if we submitted and didn't cancel!)
--- End diff --

DST should cancel -- this looks like a bug


---
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 #480: Config self reference fix

[ahgittin]

Github user ahgittin commented on the issue:

https://github.com/apache/brooklyn-server/pull/480
  
@aledsage Thanks for thorough comments.  I'll review this afternoon and 
revert with whether to merge or not.

One high level comment:  `Task` instances _should_ be left running, even if 
immediate execution is requested, because someone else might subsequently ask 
for that value in a non-immediate context.  If we cancelled after immediate 
execution then the subsequent request would fail.  This isn't a **leak** but an 
implication of supporting `Task` evaluation (and for that reason I think we 
want to prefer `TaskFactory` evaluation).  I _think_ this addresses many of 
your comments; will look closer after lunch.


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103454836
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -352,33 +357,73 @@ public T get() {
 
 //if the expected type is a closure or map and that's what we 
have, we're done (or if it's null);
 //but not allowed to return a future or DeferredSupplier as the 
resolved value
-if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v)))
+if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v) && 
!TaskFactory.class.isInstance(v)))
 return Maybe.of((T) v);
 
 try {
-if (immediately && v instanceof ImmediateSupplier) {
-final ImmediateSupplier supplier = 
(ImmediateSupplier) v;
+boolean allowImmediateExecution = false;
+boolean bailOutAfterImmediateExecution = false;
+
+if (v instanceof ImmediateSupplier) {
+allowImmediateExecution = true;
+
+} else {
+if ((v instanceof TaskFactory) && !(v instanceof 
DeferredSupplier)) {
+v = ((TaskFactory)v).newTask();
+allowImmediateExecution = true;
+bailOutAfterImmediateExecution = true;
+
BrooklynTaskTags.setTransient(((TaskAdaptable)v).asTask());
+if (isEvaluatingImmediately()) {
+// not needed if executing immediately
+BrooklynTaskTags.addTagDynamically( 
((TaskAdaptable)v).asTask(), BrooklynTaskTags.IMMEDIATE_TASK_TAG );
+}
+}
+
+//if it's a task or a future, we wait for the task to 
complete
+if (v instanceof TaskAdaptable) {
+v = ((TaskAdaptable) v).asTask();
+}
+}
+
+if (allowImmediateExecution && isEvaluatingImmediately()) {
+// TODO could allow for everything, when evaluating 
immediately -- but if the target isn't safe to run again
+// then we have to fail if immediate didn't work; to avoid 
breaking semantics we only do that for a few cases;
+// might be nice to get to the point where we can break 
those semantics however, 
+// ie weakening what getImmediate supports and making it 
be non-blocking, so that bailOut=true is the default.
+// if: v instanceof TaskFactory -- it is safe, it's a new 
API (but it is currently the only one supported);
+// more than safe, we have to do it -- or add code 
here to cancel tasks -- because it spawns new tasks
+// (other objects passed through here don't get 
cancelled, because other things might try again later;
+// ie a task or future passed in here might naturally 
be long-running so cancelling is wrong,
+// but with a task factory generated task it would 
leak if we submitted and didn't cancel!)
--- End diff --

Another failing test I wrote in `ValueResolverTest`, which leaks tasks 
(because the outer task is cancelled, while the inner tasks are not).

I'm a bit surprised that it fails - I had assumed that the `SequentialTask` 
would cancel its children when it was cancelled.

```
public void testTaskFactoryGetImmediatelyDoesNotBlockWithNestedTasks() {
final int NUM_CALLS = 3;
final AtomicInteger executingCount = new AtomicInteger();
final List outerTasks = Lists.newArrayList();

TaskFactory taskFactory = new TaskFactory() {
@Override public Task newTask() {
SequentialTask result = new 
SequentialTask<>(ImmutableList.of(new Callable() {
public String call() {
executingCount.incrementAndGet();
try {
Time.sleep(Duration.ONE_MINUTE);
return "myval";
} finally {
executingCount.decrementAndGet();
}
}}));
outerTasks.add(result);
return result;
}
};
for (int i = 0; i < NUM_CALLS; i++) {
Maybe result = 
Tasks.resolving(taskFactory).as(String.class).context(app).immediately(true).getMaybe();

[GitHub] brooklyn-server pull request #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103446823
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
 ---
@@ -96,7 +98,50 @@ public ExecutionManager getExecutionManager() {
 /** returns tasks started by this context (or tasks which have all the 
tags on this object) */
 @Override
 public Set getTasks() { return 
executionManager.getTasksWithAllTags(tags); }
- 
+
+/** performs execution without spawning a new task thread, though it 
does temporarily set a fake task for the purpose of getting context;
+ * currently supports suppliers or callables  */
+@SuppressWarnings("unchecked")
+@Override
+public  Maybe getImmediately(Object callableOrSupplier) {
+BasicTask fakeTaskForContext;
+if (callableOrSupplier instanceof BasicTask) {
+fakeTaskForContext = (BasicTask)callableOrSupplier;
--- End diff --

Under what code path do we get here? Looking at 
`ValueResolver.getMaybeInternal(...)`, `allowImmediateExecution` will be false 
if the object is a task (rather than a `TaskFactory` or `DeferredSupplier`). 

For example, the test below fails (added to `ValueResolverTest`). It leaves 
a single instance of the task executing.

```
public void testTaskGetImmediatelyDoesNotBlock() {
final AtomicInteger executingCount = new AtomicInteger();

final Task task = new BasicTask<>(new Callable() {
public String call() {
executingCount.incrementAndGet();
try {
Time.sleep(Duration.ONE_MINUTE);
return "myval";
} finally {
executingCount.decrementAndGet();
}
}});

for (int i = 0; i < 3; i++) {
Maybe result = 
Tasks.resolving(task).as(String.class).context(app).immediately(true).getMaybe();
Asserts.assertTrue(result.isAbsent(), "result="+result);
}

Asserts.assertFalse(task.isSubmitted());

// The call below default times out after 30s while the task above 
is still running
// Expect the task to not be left running.
Asserts.succeedsEventually(new Runnable() {
public void run() {
Asserts.assertEquals(executingCount.get(), 0);
}
});
}
```

I'm not convinced that we want to support setting a config key with a value 
of type `Task` (long term). But I guess that will require more work, to change 
how `DependentConfiguration` is implemented. So we do need to support it 
short-term :-(


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103439058
  
--- Diff: 
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
 ---
@@ -554,15 +562,55 @@ protected String resolveKeyName(boolean immediately) {
 .displayName("retrieving config for "+keyName)
 .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
 .dynamic(false)
-.body(new Callable() {
-@Override
-public Object call() throws Exception {
-Entity targetEntity = component.get();
-String keyNameS = resolveKeyName(true);
-ConfigKey key = 
targetEntity.getEntityType().getConfigKey(keyNameS);
-return targetEntity.getConfig(key != null ? 
key : ConfigKeys.newConfigKey(Object.class, keyNameS));
-}})
-.build();
+.body(newCallable(false)).build();
+}
+
+private Callable newCallable(final boolean immediate) {
--- End diff --

Minor: I'd have included a comment on this method to say that if immediate 
then the return value will be `Maybe` whereas if `!immediate` then the 
return value of the callable will be the actual `Object`. The difference of 
those semantics are not obvious in the method, but are relied upon by the 
caller.

It's surprising that the same method is used to return the two different 
styles of result, but I see why you did it (to reduce duplication).


---
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 #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103459565
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/feed/CommandPollConfig.java ---
@@ -0,0 +1,201 @@
+/*
+ * 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.feed;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+
+import org.apache.brooklyn.api.sensor.AttributeSensor;
+import org.apache.brooklyn.core.feed.PollConfig;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+
+public class CommandPollConfig extends PollConfig {
+
+private Supplier commandSupplier;
+private List>> dynamicEnvironmentSupplier 
= MutableList.of();
+
+public static final Predicate DEFAULT_SUCCESS = new 
Predicate() {
+@Override
+public boolean apply(@Nullable SshPollValue input) {
+return input != null && input.getExitStatus() == 0;
+}};
+
+public static  CommandPollConfig forSensor(AttributeSensor 
sensor) {
+return new CommandPollConfig(sensor);
+}
+
+public static CommandPollConfig forMultiple() {
+return new CommandPollConfig(PollConfig.NO_SENSOR);
+}
+
+public CommandPollConfig(AttributeSensor sensor) {
+super(sensor);
+super.checkSuccess(DEFAULT_SUCCESS);
+}
+
+public CommandPollConfig(CommandPollConfig other) {
+super(other);
+commandSupplier = other.commandSupplier;
+}
+
+/** @deprecated since 0.7.0; use {@link #getCommandSupplier()} and 
resolve just-in-time */
+@Deprecated
+public String getCommand() {
+return getCommandSupplier().get();
+}
+public Supplier getCommandSupplier() {
+return commandSupplier;
+}
+
+/** @deprecated since 0.7.0; use {@link #getEnvSupplier()} and resolve 
just-in-time */
+@Deprecated
+public Map getEnv() {
+return getEnvSupplier().get();
+}
+
+@SuppressWarnings("unused")
+public Supplier> getEnvSupplier() {
+if (true) return new 
CombiningEnvSupplier(dynamicEnvironmentSupplier);
+
+// TODO Kept in case it's persisted; new code will not use this.
+return new Supplier>() {
--- End diff --

Since you are renaming the class this no longer needs to be kept around. 
Could add a renaming rule for the anonymous class to `CombiningEnvSupplier`.


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103455566
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -512,6 +559,24 @@ public Object call() throws Exception {
 }
 }
 
+protected Maybe execImmediate(ExecutionContext exec, Object 
immediateSupplierOrImmediateTask) {
+Maybe result;
+try {
+result = exec.getImmediately(immediateSupplierOrImmediateTask);
+} catch (ImmediateSupplier.ImmediateUnsupportedException e) {
+return null;
+}
+// let 
InterruptingImmediateSupplier.InterruptingImmediateSupplierNotSupportedForObject
 
+// bet thrown, and caller who cares will catch that to know it can 
continue
--- End diff --

typo: `be thrown`.
And could add javadoc:
```
/**
 * @throws 
InterruptingImmediateSupplier.InterruptingImmediateSupplierNotSupportedForObject
 if ...
 */
```


---
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 #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103460271
  
--- Diff: 
software/winrm/src/main/java/org/apache/brooklyn/feed/windows/CmdFeed.java ---
@@ -0,0 +1,83 @@
+/*
+ * 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.feed.windows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import org.apache.brooklyn.feed.AbstractCommandFeed;
+import org.apache.brooklyn.feed.CommandPollConfig;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
+import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
+import org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+public class CmdFeed extends AbstractCommandFeed {
--- End diff --

Suggest naming it `WinrmFeed`, analogously to `WinrmTool`.


---
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 #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103459790
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/feed/ssh/SshPollConfig.java ---
@@ -18,183 +18,21 @@
  */
 package org.apache.brooklyn.feed.ssh;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-
-import javax.annotation.Nullable;
-
-import com.google.common.base.Objects;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Predicate;
-import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
-
 import org.apache.brooklyn.api.sensor.AttributeSensor;
-import org.apache.brooklyn.core.feed.PollConfig;
-import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.collections.MutableMap;
-
-public class SshPollConfig extends PollConfig {
-
-private Supplier commandSupplier;
-private List>> dynamicEnvironmentSupplier 
= MutableList.of();
-
-public static final Predicate DEFAULT_SUCCESS = new 
Predicate() {
-@Override
-public boolean apply(@Nullable SshPollValue input) {
-return input != null && input.getExitStatus() == 0;
-}};
-
-public static  SshPollConfig forSensor(AttributeSensor 
sensor) {
-return new SshPollConfig(sensor);
-}
-
-public static SshPollConfig forMultiple() {
-return new SshPollConfig(PollConfig.NO_SENSOR);
-}
+import org.apache.brooklyn.feed.CommandPollConfig;
 
+@Deprecated
--- End diff --

Add `@deprecated since` javadoc.


---
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 #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103460429
  
--- Diff: 
software/winrm/src/test/java/org/apache/brooklyn/feed/windows/WinRmFeedIntegrationTest.java
 ---
@@ -0,0 +1,262 @@
+/*
+ * 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.feed.windows;
+
+import com.google.common.base.Function;
+import com.google.common.base.Predicates;
+import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.brooklyn.api.entity.EntityInitializer;
+import org.apache.brooklyn.api.entity.EntityLocal;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.location.MachineProvisioningLocation;
+import org.apache.brooklyn.api.sensor.AttributeSensor;
+import org.apache.brooklyn.core.entity.Attributes;
+import org.apache.brooklyn.core.entity.EntityAsserts;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.entity.EntityInternal.FeedSupport;
+import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.feed.CommandPollConfig;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
+import org.apache.brooklyn.feed.ssh.SshValueFunctions;
+import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.text.StringFunctions;
+import org.apache.brooklyn.util.text.StringPredicates;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Test is almost identical to {@link 
org.apache.brooklyn.feed.ssh.SshFeedIntegrationTest}.
+ * To launch the test I putted in ~/.brooklyn/brooklyn.properties
--- End diff --

Typo - `put` 


---
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 #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103451547
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java ---
@@ -0,0 +1,260 @@
+/*
+ * 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.feed;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntityLocal;
+import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.feed.AbstractFeed;
+import org.apache.brooklyn.core.feed.AttributePollHandler;
+import org.apache.brooklyn.core.feed.DelegatingPollHandler;
+import org.apache.brooklyn.core.feed.Poller;
+import org.apache.brooklyn.core.location.Locations;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.brooklyn.util.time.Duration;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.SetMultimap;
+import com.google.common.collect.Sets;
+import com.google.common.reflect.TypeToken;
+
+/**
+ * Provides a feed of attribute values, by polling over ssh.
+ * 
+ * Example usage (e.g. in an entity that extends SoftwareProcessImpl):
+ * 
+ * {@code
+ * private SshFeed feed;
+ * 
+ * //@Override
+ * protected void connectSensors() {
+ *   super.connectSensors();
+ *   
+ *   feed = SshFeed.builder()
+ *   .entity(this)
+ *   .machine(mySshMachineLachine)
+ *   .poll(new CommandPollConfig(SERVICE_UP)
+ *   .command("rabbitmqctl -q status")
+ *   .onSuccess(new Function() {
+ *   public Boolean apply(SshPollValue input) {
+ * return (input.getExitStatus() == 0);
+ *   }}))
+ *   .build();
+ * }
+ * 
+ * {@literal @}Override
+ * protected void disconnectSensors() {
+ *   super.disconnectSensors();
+ *   if (feed != null) feed.stop();
+ * }
+ * }
+ * 
+ * 
+ * @author aled
+ */
+public abstract class AbstractCommandFeed extends AbstractFeed {
+
+public static final Logger log = 
LoggerFactory.getLogger(AbstractCommandFeed.class);
+
+@SuppressWarnings("serial")
+public static final ConfigKey MACHINE = 
ConfigKeys.newConfigKey(
+new TypeToken() {},
+"machine");
+
+public static final ConfigKey EXEC_AS_COMMAND = 
ConfigKeys.newBooleanConfigKey("execAsCommand");
+
+@SuppressWarnings("serial")
+public static final ConfigKey> POLLS = ConfigKeys.newConfigKey(
+new TypeToken>() {},
+"polls");
+
+public static abstract class Builder> {
+private Entity entity;
+private boolean onlyIfServiceUp = false;
+private Supplier machine;
+private Duration period = Duration.of(500, TimeUnit.MILLISECONDS);
+private boolean execAsCommand = false;
+private String uniqueTag;
+private volatile boolean built;
+
+public B entity(Entity val) {
+this.entity = val;
+return self();
+}
+public B onlyIfServiceUp() { return onlyIfServiceUp(true); }
+  

[GitHub] brooklyn-server pull request #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103455492
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java ---
@@ -0,0 +1,260 @@
+/*
+ * 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.feed;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntityLocal;
+import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.feed.AbstractFeed;
+import org.apache.brooklyn.core.feed.AttributePollHandler;
+import org.apache.brooklyn.core.feed.DelegatingPollHandler;
+import org.apache.brooklyn.core.feed.Poller;
+import org.apache.brooklyn.core.location.Locations;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.brooklyn.util.time.Duration;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.SetMultimap;
+import com.google.common.collect.Sets;
+import com.google.common.reflect.TypeToken;
+
+/**
+ * Provides a feed of attribute values, by polling over ssh.
+ * 
+ * Example usage (e.g. in an entity that extends SoftwareProcessImpl):
+ * 
+ * {@code
+ * private SshFeed feed;
+ * 
+ * //@Override
+ * protected void connectSensors() {
+ *   super.connectSensors();
+ *   
+ *   feed = SshFeed.builder()
+ *   .entity(this)
+ *   .machine(mySshMachineLachine)
+ *   .poll(new CommandPollConfig(SERVICE_UP)
+ *   .command("rabbitmqctl -q status")
+ *   .onSuccess(new Function() {
+ *   public Boolean apply(SshPollValue input) {
+ * return (input.getExitStatus() == 0);
+ *   }}))
+ *   .build();
+ * }
+ * 
+ * {@literal @}Override
+ * protected void disconnectSensors() {
+ *   super.disconnectSensors();
+ *   if (feed != null) feed.stop();
+ * }
+ * }
+ * 
+ * 
+ * @author aled
+ */
+public abstract class AbstractCommandFeed extends AbstractFeed {
+
+public static final Logger log = 
LoggerFactory.getLogger(AbstractCommandFeed.class);
+
+@SuppressWarnings("serial")
+public static final ConfigKey MACHINE = 
ConfigKeys.newConfigKey(
+new TypeToken() {},
+"machine");
+
+public static final ConfigKey EXEC_AS_COMMAND = 
ConfigKeys.newBooleanConfigKey("execAsCommand");
+
+@SuppressWarnings("serial")
+public static final ConfigKey> POLLS = ConfigKeys.newConfigKey(
+new TypeToken>() {},
+"polls");
+
+public static abstract class Builder> {
+private Entity entity;
+private boolean onlyIfServiceUp = false;
+private Supplier machine;
+private Duration period = Duration.of(500, TimeUnit.MILLISECONDS);
+private boolean execAsCommand = false;
+private String uniqueTag;
+private volatile boolean built;
+
+public B entity(Entity val) {
+this.entity = val;
+return self();
+}
+public B onlyIfServiceUp() { return onlyIfServiceUp(true); }
+  

[GitHub] brooklyn-server pull request #564: Winrm CmdFeed

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/564#discussion_r103450036
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java ---
@@ -0,0 +1,260 @@
+/*
+ * 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.feed;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntityLocal;
+import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.feed.AbstractFeed;
+import org.apache.brooklyn.core.feed.AttributePollHandler;
+import org.apache.brooklyn.core.feed.DelegatingPollHandler;
+import org.apache.brooklyn.core.feed.Poller;
+import org.apache.brooklyn.core.location.Locations;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.brooklyn.util.time.Duration;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.SetMultimap;
+import com.google.common.collect.Sets;
+import com.google.common.reflect.TypeToken;
+
+/**
+ * Provides a feed of attribute values, by polling over ssh.
+ * 
+ * Example usage (e.g. in an entity that extends SoftwareProcessImpl):
+ * 
+ * {@code
+ * private SshFeed feed;
+ * 
+ * //@Override
+ * protected void connectSensors() {
+ *   super.connectSensors();
+ *   
+ *   feed = SshFeed.builder()
+ *   .entity(this)
+ *   .machine(mySshMachineLachine)
+ *   .poll(new CommandPollConfig(SERVICE_UP)
+ *   .command("rabbitmqctl -q status")
+ *   .onSuccess(new Function() {
+ *   public Boolean apply(SshPollValue input) {
+ * return (input.getExitStatus() == 0);
+ *   }}))
+ *   .build();
+ * }
+ * 
+ * {@literal @}Override
+ * protected void disconnectSensors() {
+ *   super.disconnectSensors();
+ *   if (feed != null) feed.stop();
+ * }
+ * }
+ * 
+ * 
+ * @author aled
--- End diff --

The comment is not accurate for this class. Could replace it with 
references to the implementing classes.


---
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 #91: Updated "hello-world-sql" example webapp ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/brooklyn-library/pull/91


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103448474
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
 ---
@@ -96,7 +98,50 @@ public ExecutionManager getExecutionManager() {
 /** returns tasks started by this context (or tasks which have all the 
tags on this object) */
 @Override
 public Set getTasks() { return 
executionManager.getTasksWithAllTags(tags); }
- 
+
+/** performs execution without spawning a new task thread, though it 
does temporarily set a fake task for the purpose of getting context;
+ * currently supports suppliers or callables  */
+@SuppressWarnings("unchecked")
+@Override
+public  Maybe getImmediately(Object callableOrSupplier) {
+BasicTask fakeTaskForContext;
+if (callableOrSupplier instanceof BasicTask) {
+fakeTaskForContext = (BasicTask)callableOrSupplier;
+if (fakeTaskForContext.isQueuedOrSubmitted()) {
+if (fakeTaskForContext.isDone()) {
+return Maybe.of((T)fakeTaskForContext.getUnchecked());
+} else {
+throw new ImmediateUnsupportedException("Task is in 
progress and incomplete: "+fakeTaskForContext);
+}
+}
+callableOrSupplier = fakeTaskForContext.getJob();
--- End diff --

Very clever, getting the underlying `Callable` to execute from the job!

However, I wonder if this just reduces the chance of leaving tasks behind 
(rather than preventing it).

The test below fails when added to `ValueResolverTest` (when also including 
the change to `ValueResolver.getMaybeInternal()` to set 
`allowImmediateExecution = true` so that this code path is taken). 

```
public void testTaskGetImmediatelyDoesNotBlockWithNestedTasks() {
final AtomicInteger executingCount = new AtomicInteger();

final SequentialTask outerTask = new 
SequentialTask<>(ImmutableList.of(new Callable() {
public String call() {
executingCount.incrementAndGet();
try {
Time.sleep(Duration.ONE_MINUTE);
return "myval";
} finally {
executingCount.decrementAndGet();
}
}}));

for (int i = 0; i < 3; i++) {
Maybe result = 
Tasks.resolving(outerTask).as(String.class).context(app).immediately(true).getMaybe();
Asserts.assertTrue(result.isAbsent(), "result="+result);
}

Asserts.assertFalse(outerTask.isSubmitted());

// the call below default times out after 30s while the task above 
is still running
Asserts.succeedsEventually(new Runnable() {
public void run() {
Asserts.assertEquals(executingCount.get(), 0);
}
});
}
```


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103452425
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -352,33 +357,73 @@ public T get() {
 
 //if the expected type is a closure or map and that's what we 
have, we're done (or if it's null);
 //but not allowed to return a future or DeferredSupplier as the 
resolved value
-if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v)))
+if (v==null || (!forceDeep && type.isInstance(v) && 
!Future.class.isInstance(v) && !DeferredSupplier.class.isInstance(v) && 
!TaskFactory.class.isInstance(v)))
 return Maybe.of((T) v);
 
 try {
-if (immediately && v instanceof ImmediateSupplier) {
-final ImmediateSupplier supplier = 
(ImmediateSupplier) v;
+boolean allowImmediateExecution = false;
+boolean bailOutAfterImmediateExecution = false;
+
+if (v instanceof ImmediateSupplier) {
+allowImmediateExecution = true;
+
+} else {
+if ((v instanceof TaskFactory) && !(v instanceof 
DeferredSupplier)) {
+v = ((TaskFactory)v).newTask();
+allowImmediateExecution = true;
+bailOutAfterImmediateExecution = true;
+
BrooklynTaskTags.setTransient(((TaskAdaptable)v).asTask());
+if (isEvaluatingImmediately()) {
+// not needed if executing immediately
+BrooklynTaskTags.addTagDynamically( 
((TaskAdaptable)v).asTask(), BrooklynTaskTags.IMMEDIATE_TASK_TAG );
+}
+}
+
+//if it's a task or a future, we wait for the task to 
complete
+if (v instanceof TaskAdaptable) {
+v = ((TaskAdaptable) v).asTask();
--- End diff --

When playing around with the extra tests (see previous comments), do we 
need to add `allowImmediateExecution = true;` when `v instanceof 
TaskAdaptable`? Or will that have other unwanted? side-effects?


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103455943
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -512,6 +559,24 @@ public Object call() throws Exception {
 }
 }
 
+protected Maybe execImmediate(ExecutionContext exec, Object 
immediateSupplierOrImmediateTask) {
+Maybe result;
+try {
+result = exec.getImmediately(immediateSupplierOrImmediateTask);
+} catch (ImmediateSupplier.ImmediateUnsupportedException e) {
+return null;
--- End diff --

I'd prefer this to propagate the exception, rather than return null. I 
don't like the use of null to indicate that it couldn't be evaluated.


---
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 #480: Config self reference fix

[aledsage]

Github user aledsage commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/480#discussion_r103447467
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
 ---
@@ -96,7 +98,50 @@ public ExecutionManager getExecutionManager() {
 /** returns tasks started by this context (or tasks which have all the 
tags on this object) */
 @Override
 public Set getTasks() { return 
executionManager.getTasksWithAllTags(tags); }
- 
+
+/** performs execution without spawning a new task thread, though it 
does temporarily set a fake task for the purpose of getting context;
+ * currently supports suppliers or callables  */
+@SuppressWarnings("unchecked")
+@Override
+public  Maybe getImmediately(Object callableOrSupplier) {
+BasicTask fakeTaskForContext;
+if (callableOrSupplier instanceof BasicTask) {
+fakeTaskForContext = (BasicTask)callableOrSupplier;
--- End diff --

If I change `ValueResolver.getMaybeInternal()` to set 
`allowImmediateExecution = true;` when it is passed a `TaskAdaptable`, then the 
above test does pass.


---
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.
---

[jira] [Commented] (BROOKLYN-445) Search path and meaning of catalogItemId is overloaded

[Alex Heneveld (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887951#comment-15887951
 ] 

Alex Heneveld commented on BROOKLYN-445:


A simple fix is to prevent the call to set the catalog item ID from a context 
thread.  All tests seem to pass - see 
https://github.com/apache/brooklyn-server/pull/573 .

If this is problematic we will have to introduce a separate "searchable catalog 
item IDs" field, or, probably better, introduce and use 
`catalogItemIdsForSearching` which could be a list, including caller context.  
(This may be related to an idea for a `catalogBomId` on catalog items, so we 
know which BOM defines things, and can resolve unversioned references to the 
same BOM where an item is defined, so e.g. if my-cluster:1.0 refers to my-node, 
with my-cluster and my-node in the same bundle/bom, then my-node is resolved to 
the local bom as my-cluster, rather than the latest my-node.)


> Search path and meaning of catalogItemId is overloaded
> --
>
> Key: BROOKLYN-445
> URL: https://issues.apache.org/jira/browse/BROOKLYN-445
> Project: Brooklyn
>  Issue Type: Bug
>Affects Versions: 0.10.0
>Reporter: Alex Heneveld
>
> We use catalogItemId on a spec or BrooklynObject for two things:
> (1) Record the catalog item that was defined to create an item
> (2) Find the search path to use when looking up resources and/or creating 
> other specs
> Most of the time these two are the same, e.g. an entity comes from catalog 
> item `cassandra-node:1.0` and the implementation should use the bundles 
> defined against that to resolve scripts etc.  The catalogItemId is the only 
> record of the catalog-entry that was used to define the entity; the entity 
> spec reduces it to the java type which might tell you one bundle but won't 
> tell you of other library entries from the definition.
> However in some cases we seem to want the context catalogItemId to be 
> inherited, e.g. we reference a stock type like `VanillaServer` as a child but 
> supply config pointing at scripts/images in the parent's bundle.  This is 
> currently achieved by a line in AbstractBrooklynObjectSpec's constructor 
> which sets the catalogItemId from the thread context entity's catalog item 
> ID.  However there are a couple problems with this:
> (A) We can't tell if the `catalogItemId` is the definition of an entity (it 
> usually is, but sometimes might be inherited), so we can't tell what was used 
> to create an entity
> (B) A child's search behaviour differs depending whether that child comes 
> from another catalogItemId (which will overwrite the inherited context 
> catalogItemId) or a stock item (e.g. a java type is passed to the spec 
> constructor)
> (C) When setting the EntityDynamicType we can't tell whether to clear the 
> config keys set there; in InternalEntityFactory.addSpecParameters we need to 
> know whether the spec extends the Java type defining it (in which case CAMP 
> code in BasicSpecParameter.initializeSpecWithExplicitParameters has filtered 
> out those keys which are not type-definition inheritable and set the 
> spec.parameters, so the EntityDynamicType's keys should be cleared) or not 
> (in which case spec.parameters won't normally be set and EDT.configKeys 
> should not be cleared).  Currently it checks whether there is a 
> catalogItemId, and assume it is set iff the spec is extending (former case); 
> this assumption fails if catalogItemId is inherited from context.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (BROOKLYN-445) Search path and meaning of catalogItemId is overloaded

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/BROOKLYN-445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15887941#comment-15887941
 ] 

ASF GitHub Bot commented on BROOKLYN-445:
-

GitHub user ahgittin opened a pull request:

https://github.com/apache/brooklyn-server/pull/573

Do not runtime-inherit catalog item

fixes https://issues.apache.org/jira/browse/BROOKLYN-445 and failing tests 
included here

means catalog BOMs should be written such that any entity/policy/etc which 
wants to access resources in the bundle is defined as its own item

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ahgittin/brooklyn-server 
do-not-inherit-catalog-item-id

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/573.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 #573


commit d7cbaeed46d7fedf8e49df015baa812352060f45
Author: Alex Heneveld 
Date:   2017-02-28T12:43:14Z

add original failing test from @sjcorbett

for failure to set inherited config keys - 
https://issues.apache.org/jira/browse/BROOKLYN-445

commit edaf05441ee46d05e4479a79aee055e6cbfe1237
Author: Alex Heneveld 
Date:   2017-02-28T12:44:17Z

add similar tests, including one failing, isolating the error

commit 9c38c94eeeb5a3de2a082f81546440980b1abb5c
Author: Alex Heneveld 
Date:   2017-02-28T12:55:24Z

apply the fix for previous failing tests and 
https://issues.apache.org/jira/browse/BROOKLYN-445

do not use a context catalog item id.  this is a significant change and 
release notes should be updated,
but it doesn't seem to break any of our tests; presumably most catalog 
items are already refactoring
so each item is declared as its own item.




> Search path and meaning of catalogItemId is overloaded
> --
>
> Key: BROOKLYN-445
> URL: https://issues.apache.org/jira/browse/BROOKLYN-445
> Project: Brooklyn
>  Issue Type: Bug
>Affects Versions: 0.10.0
>Reporter: Alex Heneveld
>
> We use catalogItemId on a spec or BrooklynObject for two things:
> (1) Record the catalog item that was defined to create an item
> (2) Find the search path to use when looking up resources and/or creating 
> other specs
> Most of the time these two are the same, e.g. an entity comes from catalog 
> item `cassandra-node:1.0` and the implementation should use the bundles 
> defined against that to resolve scripts etc.  The catalogItemId is the only 
> record of the catalog-entry that was used to define the entity; the entity 
> spec reduces it to the java type which might tell you one bundle but won't 
> tell you of other library entries from the definition.
> However in some cases we seem to want the context catalogItemId to be 
> inherited, e.g. we reference a stock type like `VanillaServer` as a child but 
> supply config pointing at scripts/images in the parent's bundle.  This is 
> currently achieved by a line in AbstractBrooklynObjectSpec's constructor 
> which sets the catalogItemId from the thread context entity's catalog item 
> ID.  However there are a couple problems with this:
> (A) We can't tell if the `catalogItemId` is the definition of an entity (it 
> usually is, but sometimes might be inherited), so we can't tell what was used 
> to create an entity
> (B) A child's search behaviour differs depending whether that child comes 
> from another catalogItemId (which will overwrite the inherited context 
> catalogItemId) or a stock item (e.g. a java type is passed to the spec 
> constructor)
> (C) When setting the EntityDynamicType we can't tell whether to clear the 
> config keys set there; in InternalEntityFactory.addSpecParameters we need to 
> know whether the spec extends the Java type defining it (in which case CAMP 
> code in BasicSpecParameter.initializeSpecWithExplicitParameters has filtered 
> out those keys which are not type-definition inheritable and set the 
> spec.parameters, so the EntityDynamicType's keys should be cleared) or not 
> (in which case spec.parameters won't normally be set and EDT.configKeys 
> should not be cleared).  Currently it checks whether there is a 
> catalogItemId, and assume it is set iff the spec is extending (former case); 
> this assumption fails if catalogItemId is inherited from context.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] brooklyn-server pull request #574: don't NPE fail is application is GC'd at ...

[ahgittin]

GitHub user ahgittin opened a pull request:

https://github.com/apache/brooklyn-server/pull/574

don't NPE fail is application is GC'd at same time as location

i think we want to make sure a record is generated, even if it is incomplete

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ahgittin/brooklyn-server 
fix-npe-in-location-usage

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/574.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 #574


commit 43a456f97bf39faa409c88e431382c60a25ff724
Author: Alex Heneveld 
Date:   2017-02-28T12:57:12Z

could get NPE if application was GC'd at same time as location, do not fail 
in that case

we want to make sure a record is generated, even if it is incomplete




---
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 #573: Do not runtime-inherit catalog item

[ahgittin]

GitHub user ahgittin opened a pull request:

https://github.com/apache/brooklyn-server/pull/573

Do not runtime-inherit catalog item

fixes https://issues.apache.org/jira/browse/BROOKLYN-445 and failing tests 
included here

means catalog BOMs should be written such that any entity/policy/etc which 
wants to access resources in the bundle is defined as its own item

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ahgittin/brooklyn-server 
do-not-inherit-catalog-item-id

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/573.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 #573


commit d7cbaeed46d7fedf8e49df015baa812352060f45
Author: Alex Heneveld 
Date:   2017-02-28T12:43:14Z

add original failing test from @sjcorbett

for failure to set inherited config keys - 
https://issues.apache.org/jira/browse/BROOKLYN-445

commit edaf05441ee46d05e4479a79aee055e6cbfe1237
Author: Alex Heneveld 
Date:   2017-02-28T12:44:17Z

add similar tests, including one failing, isolating the error

commit 9c38c94eeeb5a3de2a082f81546440980b1abb5c
Author: Alex Heneveld 
Date:   2017-02-28T12:55:24Z

apply the fix for previous failing tests and 
https://issues.apache.org/jira/browse/BROOKLYN-445

do not use a context catalog item id.  this is a significant change and 
release notes should be updated,
but it doesn't seem to break any of our tests; presumably most catalog 
items are already refactoring
so each item is declared as its own item.




---
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.
---

[jira] [Created] (BROOKLYN-446) Constructor ScheduledTask(Map, Task) is broken

[Svetoslav Neykov (JIRA)]

Svetoslav Neykov created BROOKLYN-446:
-

 Summary: Constructor ScheduledTask(Map, Task) is broken
 Key: BROOKLYN-446
 URL: https://issues.apache.org/jira/browse/BROOKLYN-446
 Project: Brooklyn
  Issue Type: Bug
Affects Versions: 0.10.0
Reporter: Svetoslav Neykov


The resulting task is executed only once, not rescheduled.
A test case to show the behaviour:

{noformat}
import static org.testng.Assert.assertTrue;

import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;

import org.apache.brooklyn.api.mgmt.Task;
import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.time.Duration;
import org.testng.annotations.Test;

import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.Runnables;

public class ScheduledTaskTest extends BrooklynMgmtUnitTestSupport {
/**
 * Fails with:
 * java.lang.UnsupportedOperationException
 * at 
com.google.common.collect.ImmutableMap.remove(ImmutableMap.java:338)
 * at 
org.apache.brooklyn.util.core.task.BasicTask.(BasicTask.java:115)
 * at 
org.apache.brooklyn.util.core.task.BasicTask.(BasicTask.java:107)
 * at 
org.apache.brooklyn.util.core.task.ScheduledTask.(ScheduledTask.java:96)
 * at 
org.apache.brooklyn.util.core.task.ScheduledTask.(ScheduledTask.java:88)
 * at 
org.apache.brooklyn.util.core.task.ScheduledTaskTest.testImmutableFlags(ScheduledTaskTest.java:43)
 */
@Test
public void testImmutableFlags() {
Map flags = ImmutableMap.of(
"period", Duration.ONE_SECOND,
"delay", Duration.ONE_SECOND);
Task task = new BasicTask<>(Runnables.doNothing());
new ScheduledTask(flags, task);
}

/**
 * Fails with:
 * java.lang.AssertionError: failed succeeds-eventually, 69 attempts, 
30005ms elapsed: AssertionError: expected [true] but found [false]
 * at 
org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:1008)
 * at 
org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:895)
 * at 
org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:888)
 * at 
org.apache.brooklyn.util.core.task.ScheduledTaskTest.testTaskConstructor(ScheduledTaskTest.java:60)
 */
@Test
public void testTaskConstructor() {
final AtomicInteger cnt = new AtomicInteger();
Map flags = MutableMap.of(
"period", Duration.ONE_SECOND,
"delay", Duration.ONE_SECOND);
Task task = new BasicTask<>(new Runnable() {
@Override
public void run() {
cnt.incrementAndGet();
}
});
ScheduledTask scheduledTask = new ScheduledTask(flags, task);
mgmt.getExecutionManager().submit(scheduledTask);
Asserts.succeedsEventually(new Runnable() {
@Override
public void run() {
assertTrue(cnt.get() > 1);
}
});
}
}
{noformat}


The problem is that the task gets reused and on the second schedule attempt it 
already has a future assigned so is not scheduled any more.
The best solution seems to be to remove the constructor altogether. Optionally 
providing a Runnable/Callable job constructor, wrapping the job in a factory, 
returning a new task on each submission.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Created] (BROOKLYN-445) Search path and meaning of catalogItemId is overloaded

[Alex Heneveld (JIRA)]

Alex Heneveld created BROOKLYN-445:
--

 Summary: Search path and meaning of catalogItemId is overloaded
 Key: BROOKLYN-445
 URL: https://issues.apache.org/jira/browse/BROOKLYN-445
 Project: Brooklyn
  Issue Type: Bug
Affects Versions: 0.10.0
Reporter: Alex Heneveld


We use catalogItemId on a spec or BrooklynObject for two things:

(1) Record the catalog item that was defined to create an item
(2) Find the search path to use when looking up resources and/or creating other 
specs

Most of the time these two are the same, e.g. an entity comes from catalog item 
`cassandra-node:1.0` and the implementation should use the bundles defined 
against that to resolve scripts etc.  The catalogItemId is the only record of 
the catalog-entry that was used to define the entity; the entity spec reduces 
it to the java type which might tell you one bundle but won't tell you of other 
library entries from the definition.

However in some cases we seem to want the context catalogItemId to be 
inherited, e.g. we reference a stock type like `VanillaServer` as a child but 
supply config pointing at scripts/images in the parent's bundle.  This is 
currently achieved by a line in AbstractBrooklynObjectSpec's constructor which 
sets the catalogItemId from the thread context entity's catalog item ID.  
However there are a couple problems with this:

(A) We can't tell if the `catalogItemId` is the definition of an entity (it 
usually is, but sometimes might be inherited), so we can't tell what was used 
to create an entity

(B) A child's search behaviour differs depending whether that child comes from 
another catalogItemId (which will overwrite the inherited context 
catalogItemId) or a stock item (e.g. a java type is passed to the spec 
constructor)

(C) When setting the EntityDynamicType we can't tell whether to clear the 
config keys set there; in InternalEntityFactory.addSpecParameters we need to 
know whether the spec extends the Java type defining it (in which case CAMP 
code in BasicSpecParameter.initializeSpecWithExplicitParameters has filtered 
out those keys which are not type-definition inheritable and set the 
spec.parameters, so the EntityDynamicType's keys should be cleared) or not (in 
which case spec.parameters won't normally be set and EDT.configKeys should not 
be cleared).  Currently it checks whether there is a catalogItemId, and assume 
it is set iff the spec is extending (former case); this assumption fails if 
catalogItemId is inherited from context.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] brooklyn-server pull request #572: Increase timeout in ReachableSocketFinder...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/brooklyn-server/pull/572


---
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 #572: Increase timeout in ReachableSocketFinderTest

[geomacy]

Github user geomacy commented on the issue:

https://github.com/apache/brooklyn-server/pull/572
  
+1 Looks good to me, 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.
---