You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/11/09 11:41:47 UTC
[GitHub] brooklyn-server pull request #887: Config on entity dynamic goes on type aft...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/887
Config on entity dynamic goes on type after persistence
Fixes two issues noted with rebind of dynamic config. See first commit first for failing test, then the others which fix it and comment.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server config-on-entity-dynamic-goes-on-type-after-persistence
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/887.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 #887
----
commit d353d2352e746339ad546758e87cb438427234ce
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-11-09T10:20:03Z
failing test for config persistence edge case
commit 82dfdd27ba34d13b8bdb3fcd9a5d84f0a463bf09
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-11-09T10:33:38Z
compare query and declared types when looking up config
fixes one of the two checks in the previously failing test by
preferring the query type if more specific when looking up config.
also introduces a warning if the query type is incompatible.
commit 1980b005a4081d7374af90ac2454f51d1763d20b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-11-09T11:12:17Z
don't persist anonymous keys
another way of fixing the failing-before-this-commit test.
adding anonymous keys to the type means rebinded entities differ from original.
see comments, we still have that discrepancy for non-anonymous keys, but there it seems more acceptable.
commit 31ff1855fbd0334457e64b621376cd349c545992
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-11-09T11:34:20Z
more comments and test for persisted non-type config
in particular the fact that such added config shifts to be on the type after rebind,
and tidy for other affected tests
----
---
[GitHub] brooklyn-server pull request #887: Config on entity dynamic goes on type aft...
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/887#discussion_r150017538
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java ---
@@ -163,8 +165,27 @@ protected BasicEntityMemento(Builder builder) {
if (configByKey != null) {
for (Map.Entry<ConfigKey<?>, Object> entry : configByKey.entrySet()) {
ConfigKey<?> key = entry.getKey();
- if (!configKeys.containsKey(key.getName()) && key != staticConfigKeys.get(key.getName())) {
- configKeys.put(key.getName(), key);
+ ConfigKey<?> staticKey = staticConfigKeys.get(key.getName());
+ if (!configKeys.containsKey(key.getName()) && key != staticKey) {
+ // user added a key (programmatically) not declared on the type; persist if not anonymous.
+ // on rebind this shows up in getEntityType() which is still a difference in the pre-rebind entity,
+ // but that's more understandable and has been the case for a long time.
+ // (the entity type is unclear in the yaml era anyway.)
+ // see RebindEntityTest.test*Key*
+ if (isAnonymous(key)) {
+ // 2017-11 no longer persist these, wasteful and unhelpful,
+ // (can hurt if someone expects declared key to be meaningful)
+ log.debug("Skipping persistence of "+key+" on "+getId()+" because it is anonymous");
--- End diff --
I'm torn whether this should be `trace` instead - we create a `BasicEntityMemento` a lot. This could fill the log with 1000s of such log messages per minute if we have a lot of entities and their sensor values change regularly due to polling for performance metrics etc.
I'd either change it to trace, or guard it to only log once per type+key pair, or some such.
---
[GitHub] brooklyn-server pull request #887: Config on entity dynamic goes on type aft...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/887
---
[GitHub] brooklyn-server issue #887: Config on entity dynamic goes on type after pers...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/887
agree with comment - addressed, merging
---