You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by kiuby88 <gi...@git.apache.org> on 2016/05/20 09:59:02 UTC
[GitHub] brooklyn-server pull request: Avoiding object hashcode in entities...
GitHub user kiuby88 opened a pull request:
https://github.com/apache/brooklyn-server/pull/146
Avoiding object hashcode in entities' toString.
Fixing [BROOKLYN-271](https://issues.apache.org/jira/browse/BROOKLYN-271)
Avoiding object references in catalog ConfigKey cards, for example:
<img width="1036" alt="captura de pantalla 2016-05-19 a la s 17 46 04" src="https://cloud.githubusercontent.com/assets/3704424/15399867/992624fa-1de9-11e6-9e21-b9efe659ab1e.png">
Last picture shows that default value of `softwareProcess.lifecycleTasks` config key does not contain the object hashcode `@XXXXX`. Then, objects `toString` methods are modified to avoid the hashcode.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kiuby88/brooklyn-server BROOKLYN-271
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/146.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 #146
----
commit 5e1517b4c99f99c5368936a9f00ea180d0972776
Author: Kiuby88 <ki...@gmail.com>
Date: 2016-05-20T09:56:59Z
Avoiding object hashcode in entities' toString.
----
---
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 #146: Fix BROOKLYN-271: Avoiding object hashcode in en...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/146
LGTM, thanks @kiuby88 - merging. I incorrectly marked BROOKLYN-271 as resolved before this PR (and https://github.com/apache/brooklyn-library/pull/38) were merged - sorry about that, and thanks for spotting!
---
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: Fix BROOKLYN-271: Avoiding object ha...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/brooklyn-server/pull/146#issuecomment-222310983
Thanks @kiuby88 - agreed this will improve the auto-generated entity docs (as per BROOKLYN-271).
In general, I like the toString to give more specifics about the object where possible. That is possible for some of these, and not for others. I'll comment on individual methods where I think we could include more.
---
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: Avoiding object hashcode in entities...
Posted by kiuby88 <gi...@git.apache.org>.
Github user kiuby88 commented on the pull request:
https://github.com/apache/brooklyn-server/pull/146#issuecomment-220565759
@aledsage I think it should be better than https://github.com/apache/brooklyn-docs/pull/68
---
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 #146: Fix BROOKLYN-271: Avoiding object hashcod...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/146
---
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: Fix BROOKLYN-271: Avoiding object ha...
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/146#discussion_r64989547
--- Diff: api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java ---
@@ -432,5 +432,10 @@ public Entity getParent() {
private void checkMutable() {
if (immutable) throw new IllegalStateException("Cannot modify immutable entity spec "+this);
}
-
+
+ @Override
+ public String toString(){
+ return getClass().getName();
--- End diff --
Interestingly, there is already a `toString`, inherited from `AbstractBrooklynObjectSpec`. That will include the entity type, and will append `+"@"+Integer.toHexString(System.identityHashCode(this)`!
I can see why we might want the identityHashCode (so that when looking at the logs we can tell if something is using the same instance). But on-balance, I think it's fine to not include it. Therefore, instead of this impl I suggest you change `AbstractBrooklynObjectSpec` to be:
@Override
public String toString() {
return Objects.toStringHelper(this).omitNullValues()
.add("type", type)
.add("displayName", displayName)
.toString();
}
---
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: Fix BROOKLYN-271: Avoiding object ha...
Posted by kiuby88 <gi...@git.apache.org>.
Github user kiuby88 commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/146#discussion_r65141146
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BrooklynImageChooser.java ---
@@ -371,7 +371,12 @@ public ImageChooserFromOrdering cloneFor(ConfigBag config) {
} else {
return this;
}
- }
+ }
+
+ @Override
+ public String toString(){
+ return getClass().getName();
+ }
--- End diff --
@aledsage, this tries to avoid the following entry in the catalog for `JcloudsLocation`.
<img width="962" alt="captura de pantalla 2016-05-31 a la s 10 24 10" src="https://cloud.githubusercontent.com/assets/3704424/15668131/002369d6-271a-11e6-9134-67df64155a9e.png">
I think this impl is enough, maybe [ordering](https://github.com/apache/brooklyn-server/blob/master/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/BrooklynImageChooser.java#L330)?
---
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: Fix BROOKLYN-271: Avoiding object ha...
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/146#discussion_r64989514
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/zoneaware/ProportionalZoneFailureDetector.java ---
@@ -56,4 +56,10 @@ protected boolean doHasFailed(Location loc, ZoneHistory zoneHistory) {
return numDatapoints >= minDatapoints && proportionFailure >= maxProportionFailures;
}
}
+
+ @Override
+ public String toString(){
+ return getClass().getName();
--- End diff --
I'd go for the code below (using `com.google.common.base.Objects`):
@Override
public String toString() {
return Objects.toStringHelper(this)
.add("minDatapoints", minDatapoints)
.add("timeToConsider", timeToConsider)
.add("maxProportionFailures", maxProportionFailures)
.toString();
}
---
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.
---