You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2015/05/22 18:33:12 UTC
[GitHub] incubator-brooklyn pull request: Jclouds location resolver
GitHub user sjcorbett opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/655
Jclouds location resolver
A handful of disparate additions:
* Adjusts `JcloudsLocation` and its resolver so that it is easier to subclass.
* Exposes the predicate used to find machines in `JcloudsLocation.rebind` for customisation by subclasses.
* Adjusts the web server's logging filter to log requests that take a long time to complete.
* Adjusts EffectorUtils to rethrow `PropagatedRuntimeExceptions` rather than wrapping them in more exceptions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn jclouds-location-resolver
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/655.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 #655
----
commit dd24a4fb557d384b862364c996faa7ab2bcebdb5
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-14T18:47:34Z
EffectorUtils rethrows PropagatedRuntimeExceptions where possible
Rather than wrapping in more PropagatedRuntimeExceptions
commit 63e78d2b408ac8da5eec360609b79dcb4989dff2
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-14T18:50:52Z
PropagatedRuntimeException logs (at trace) if it's wrapping itself
commit bec410a501d923ec488c56a52729b3187b823804
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-18T11:08:31Z
Move util test specs to util/
commit 1d07bd41ec5382308a343045a4e86d6d1c20bf93
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-18T11:36:58Z
Fix router-spec for browser.
commit 69edff2d7db64b14d941a5d9c1c9b3f1ac88b1d6
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-18T14:30:09Z
Rejig LoggingFilter.
Log requests that take longer than five seconds and log all non-HEAD/GET
requests (rather than only log POST, PUT and DELETE).
commit f316025ddabef40199d845e9ad2f767823fc4edb
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-22T14:32:11Z
JcloudsLocationResolver extensible
commit aeefae22e2f756dae497efd8ecf49450251fd1e6
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-22T16:25:29Z
Expose predicate for finding machines when rebinding
Exposes the predicate to subclasses of 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: Jclouds location resolver
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/655#issuecomment-106291408
lots of good stuff here. a few minor comments, then good to merge i think.
---
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: Jclouds location resolver
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/655#discussion_r31324402
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationResolver.java ---
@@ -50,10 +50,10 @@
public static final Logger log = LoggerFactory.getLogger(JcloudsLocationResolver.class);
- public static final String JCLOUDS = "jclouds";
+ private static final String JCLOUDS = "jclouds";
- public static final Map<String,ProviderMetadata> PROVIDERS = getProvidersMap();
- public static final Map<String,ApiMetadata> APIS = getApisMap();
+ private static final Map<String,ProviderMetadata> PROVIDERS = getProvidersMap();
+ private static final Map<String,ApiMetadata> APIS = getApisMap();
--- End diff --
I don't see much reason for them to be public. Protected would make more sense.
---
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: Jclouds location resolver
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/655
---
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: Jclouds location resolver
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/655#discussion_r31225911
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationResolver.java ---
@@ -50,10 +50,10 @@
public static final Logger log = LoggerFactory.getLogger(JcloudsLocationResolver.class);
- public static final String JCLOUDS = "jclouds";
+ private static final String JCLOUDS = "jclouds";
- public static final Map<String,ProviderMetadata> PROVIDERS = getProvidersMap();
- public static final Map<String,ApiMetadata> APIS = getApisMap();
+ private static final Map<String,ProviderMetadata> PROVIDERS = getProvidersMap();
+ private static final Map<String,ApiMetadata> APIS = getApisMap();
--- End diff --
extensions might be accessing these. if they need to be private then let's go through a deprecation cycle.
---
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: Jclouds location resolver
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/655#issuecomment-104710578
Build server issue: `java.lang.RuntimeException: unable to find a free port at or above 8080`.
---
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: Jclouds location resolver
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/655#discussion_r31324334
--- Diff: locations/jclouds/src/test/java/brooklyn/location/jclouds/BrooklynMachinePoolLiveTest.java ---
@@ -98,6 +98,6 @@ public void buildClaimAndDestroy() {
private JcloudsLocation resolve(String spec) {
- return (JcloudsLocation) managementContext.getLocationRegistry().resolve(JcloudsLocationResolver.JCLOUDS+":"+spec);
+ return (JcloudsLocation) managementContext.getLocationRegistry().resolve("jclouds:"+spec);
--- End diff --
There are so many classes that hardcode "jclouds:..." that I think it's a minor one, but I'll switch it for `new JcloudsLocationResolver().getPrefix()`
---
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: Jclouds location resolver
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/655
---
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: Jclouds location resolver
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/655#discussion_r31225820
--- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -1540,6 +1540,58 @@ public JcloudsSshMachineLocation rebindMachine(Map<?,?> flags, NodeMetadata meta
}
/**
+ * @return a predicate that returns true if a {@link ComputeMetadata} instance is suitable for
+ * rebinding to given the configuration in {@link ConfigBag config}.
+ */
+ protected Predicate<ComputeMetadata> getComputeMetadataPredicate(ConfigBag config) {
+ final String rawId = (String) config.getStringKey("id");
+ final String rawHostname = (String) config.getStringKey("hostname");
+ final String rawRegion = (String) config.getStringKey("region");
+ return new Predicate<ComputeMetadata>() {
--- End diff --
if this would ever be used in config then we should avoid anonymous 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Jclouds location resolver
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/655#discussion_r31225969
--- Diff: locations/jclouds/src/test/java/brooklyn/location/jclouds/BrooklynMachinePoolLiveTest.java ---
@@ -98,6 +98,6 @@ public void buildClaimAndDestroy() {
private JcloudsLocation resolve(String spec) {
- return (JcloudsLocation) managementContext.getLocationRegistry().resolve(JcloudsLocationResolver.JCLOUDS+":"+spec);
+ return (JcloudsLocation) managementContext.getLocationRegistry().resolve("jclouds:"+spec);
--- End diff --
variable reference is better than hard-coded string i think
---
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: Jclouds location resolver
Posted by sjcorbett <gi...@git.apache.org>.
GitHub user sjcorbett reopened a pull request:
https://github.com/apache/incubator-brooklyn/pull/655
Jclouds location resolver
A handful of disparate additions:
* Adjusts `JcloudsLocation` and its resolver so that it is easier to subclass.
* Exposes the predicate used to find machines in `JcloudsLocation.rebind` for customisation by subclasses.
* Adjusts the web server's logging filter to log requests that take a long time to complete.
* Adjusts EffectorUtils to rethrow `PropagatedRuntimeExceptions` rather than wrapping them in more exceptions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn jclouds-location-resolver
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/655.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 #655
----
commit dd24a4fb557d384b862364c996faa7ab2bcebdb5
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-14T18:47:34Z
EffectorUtils rethrows PropagatedRuntimeExceptions where possible
Rather than wrapping in more PropagatedRuntimeExceptions
commit 63e78d2b408ac8da5eec360609b79dcb4989dff2
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-14T18:50:52Z
PropagatedRuntimeException logs (at trace) if it's wrapping itself
commit bec410a501d923ec488c56a52729b3187b823804
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-18T11:08:31Z
Move util test specs to util/
commit 1d07bd41ec5382308a343045a4e86d6d1c20bf93
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-18T11:36:58Z
Fix router-spec for browser.
commit 69edff2d7db64b14d941a5d9c1c9b3f1ac88b1d6
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-18T14:30:09Z
Rejig LoggingFilter.
Log requests that take longer than five seconds and log all non-HEAD/GET
requests (rather than only log POST, PUT and DELETE).
commit f316025ddabef40199d845e9ad2f767823fc4edb
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-22T14:32:11Z
JcloudsLocationResolver extensible
commit aeefae22e2f756dae497efd8ecf49450251fd1e6
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-05-22T16:25:29Z
Expose predicate for finding machines when rebinding
Exposes the predicate to subclasses of 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: Jclouds location resolver
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/655#issuecomment-106837293
@ahgittin Thanks
---
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.
---