You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by grkvlt <gi...@git.apache.org> on 2016/11/06 01:42:50 UTC
[GitHub] brooklyn-server pull request #408: [TMP] Use actual config key if available ...
GitHub user grkvlt opened a pull request:
https://github.com/apache/brooklyn-server/pull/408
[TMP] Use actual config key if available in DSL
Attempting to fix issue where config defined in `brooklyn.parameters` is resolved as `null` instead of using the default value, when used inside a transformer. This appears to be a race condition or related to use of `getImmediately()` since the issue disappears when investigated with a debugger.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/brooklyn-server transformer-config-nulls
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/408.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 #408
----
commit 3d8c3d064a3bacb2fcada70799d86a2ca42cd7a0
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-11-06T01:40:38Z
Use actual config key if available in DSL
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #408: Use actual config key if available in DSL
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/408
@aledsage your #410 looks good, so I will close this. I had not thought about the semantics of `get()` vs. `getImmediately()` and `getRaw()` etc. but the rationale for just checking for `Maybe.absent()` seems sound.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #408: Use actual config key if available in DSL
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/408#discussion_r86738440
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java ---
@@ -257,7 +260,7 @@ public EntityId(DslComponent component) {
@Override
public Maybe<Object> getImmediately() {
Maybe<Entity> targetEntityMaybe = component.getImmediately();
- if (targetEntityMaybe.isAbsent()) return Maybe.absent("Target entity not available");
+ if (targetEntityMaybe.isAbsentOrNull()) return Maybe.absent("Target entity not available");
--- End diff --
As above: this is changing the semantics of `getImmediately` compared to `get`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #408: Use actual config key if available in DSL
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt closed the pull request at:
https://github.com/apache/brooklyn-server/pull/408
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #408: Use actual config key if available in DSL
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/408
@grkvlt thanks for finding and fixing this!
See https://github.com/apache/brooklyn-server/pull/410 - I've extracted your fix for the problem from this PR, and added an additonal test. Happy to merge that as soon as jenkins confirms it works.
For the changes to `isAbsentOrNull`, see my comments. I believe that is what is making `RelativeEntityTestCaseTest` fail in jenkins (it fails for me locally as well with your branch, but does not fail with PR #410).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #408: Use actual config key if available in DSL
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/408
Failures look related, I can see the same problems as https://builds.apache.org/job/brooklyn-server-pull-requests/1283/org.apache.brooklyn$brooklyn-test-framework/testReport/ when running `mvn clean install` locally.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #408: Use actual config key if available in DSL
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/408#discussion_r86738337
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java ---
@@ -144,7 +147,7 @@ public Entity call() throws Exception {
protected Maybe<Entity> callImpl(boolean immediate) throws Exception {
Maybe<Entity> entityMaybe = getEntity(immediate);
- if (immediate && entityMaybe.isAbsent()) {
+ if (immediate && entityMaybe.isAbsentOrNull()) {
--- End diff --
Is this to avoid the `NullPointerException` you saw before https://github.com/apache/brooklyn-server/pull/404 was merged? I wondered about this.
The problem with returning `Maybe.of(null)` if the entity can't be resolved is that it's different semantics from `deferredSupplier.get()`. For the latter, it would throw `NoSuchElementException` with a nice message explaining it. Now this will just return a null.
The intent is that `getImmediately()` will either return `absent()` if it can't be resolved immediately, or will return immediately exactly the same value as `get()` would.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---