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 2015/06/02 12:58:36 UTC
[GitHub] incubator-brooklyn pull request: Remove shared statics causing iss...
GitHub user grkvlt opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/671
Remove shared statics causing issues in Clocker
Clocker has more than one `JcloudsLocation` so the check for exceptions is skipped sometimes, becasue the guard is a static variable shared by all instances. This gives unhelpful error messages occasionally, particularly in the presence of network failures raising `IOException`s.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/incubator-brooklyn fix/multiple-jclouds-locations-exception
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/671.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 #671
----
commit 03a35bbe4f4b12ebdc189869b1ea8eb3207a4777
Author: Andrew Kennedy <gr...@apache.org>
Date: 2015-06-02T10:51:06Z
Remove shared statics causing issues in Clocker with more than one JcloudsLocation
----
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#discussion_r31581298
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -1502,8 +1504,7 @@ protected UserCreation createUserStatements(@Nullable Image image, ConfigBag con
* if this config key is not set, use a key `brooklyn_id_rsa` and `.pub` in `MGMT_BASE_DIR`,
* with permission 0600, creating it if necessary, and logging the fact that this was created.
*/
- if (!loggedSshKeysHint && !config.containsKey(PRIVATE_KEY_FILE)) {
- loggedSshKeysHint = true;
+ if (loggedSshKeysHint.compareAndSet(false, true) && !config.containsKey(PRIVATE_KEY_FILE)) {
--- End diff --
Right, yes...
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#discussion_r31581556
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -204,7 +205,8 @@
private static final Pattern LIST_PATTERN = Pattern.compile("^\\[(.*)\\]$");
private static final Pattern INTEGER_PATTERN = Pattern.compile("^\\d*$");
- private static boolean loggedSshKeysHint = false;
+ private final AtomicBoolean loggedSshKeysHint = new AtomicBoolean(false);
+ private final AtomicBoolean listedAvailableTemplatesOnNoSuchTemplate = new AtomicBoolean(false);
--- End diff --
a guava `Cache` is maybe the hyper-engineered way but i think tying it to the `JcloudsLocation` instance is a pragmatic step. the `JcloudsLocation` instance should be cached, i *think*, and if not all we do is log some debug/info a bit 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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#discussion_r31581287
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -204,7 +205,8 @@
private static final Pattern LIST_PATTERN = Pattern.compile("^\\[(.*)\\]$");
private static final Pattern INTEGER_PATTERN = Pattern.compile("^\\d*$");
- private static boolean loggedSshKeysHint = false;
+ private final AtomicBoolean loggedSshKeysHint = new AtomicBoolean(false);
+ private final AtomicBoolean listedAvailableTemplatesOnNoSuchTemplate = new AtomicBoolean(false);
--- End diff --
Yes, I wasn't sure what to do to achieve the actual intent. `ThreadLocal` wouldn't work, maybe a `Map` with the location ID as the key? I wasn't sure whether the IDs would be the same for two `Location` instances generated by the management context for the same underlying location?
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#discussion_r31581105
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -204,7 +205,8 @@
private static final Pattern LIST_PATTERN = Pattern.compile("^\\[(.*)\\]$");
private static final Pattern INTEGER_PATTERN = Pattern.compile("^\\d*$");
- private static boolean loggedSshKeysHint = false;
+ private final AtomicBoolean loggedSshKeysHint = new AtomicBoolean(false);
+ private final AtomicBoolean listedAvailableTemplatesOnNoSuchTemplate = new AtomicBoolean(false);
--- End diff --
`JcloudsLocation` instances do tend to be re-used where it's the same location so this seems like a good idea to me; means we might log more than we should, but that's better than the previous which logged less than we should!
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#issuecomment-108126872
one comment of note -- the `&&` args order -- then good to merge
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/671
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#discussion_r31582591
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -204,7 +205,8 @@
private static final Pattern LIST_PATTERN = Pattern.compile("^\\[(.*)\\]$");
private static final Pattern INTEGER_PATTERN = Pattern.compile("^\\d*$");
- private static boolean loggedSshKeysHint = false;
+ private final AtomicBoolean loggedSshKeysHint = new AtomicBoolean(false);
+ private final AtomicBoolean listedAvailableTemplatesOnNoSuchTemplate = new AtomicBoolean(false);
--- End diff --
Can look into that in a further PR, though...
---
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] incubator-brooklyn pull request: Remove shared statics causing iss...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/671#discussion_r31581022
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -1502,8 +1504,7 @@ protected UserCreation createUserStatements(@Nullable Image image, ConfigBag con
* if this config key is not set, use a key `brooklyn_id_rsa` and `.pub` in `MGMT_BASE_DIR`,
* with permission 0600, creating it if necessary, and logging the fact that this was created.
*/
- if (!loggedSshKeysHint && !config.containsKey(PRIVATE_KEY_FILE)) {
- loggedSshKeysHint = true;
+ if (loggedSshKeysHint.compareAndSet(false, true) && !config.containsKey(PRIVATE_KEY_FILE)) {
--- End diff --
`&&` args should be the other way round so that boolean is only set true if message is logged
---
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.
---