You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by drigodwin <gi...@git.apache.org> on 2017/01/06 15:21:43 UTC
[GitHub] brooklyn-server pull request #508: Alternate names for JcloudsProviders
GitHub user drigodwin opened a pull request:
https://github.com/apache/brooklyn-server/pull/508
Alternate names for JcloudsProviders
This adds the ability to have alternate names for JcloudsProviders stored a property file `org/apache/brooklyn/core/mgmt/persist/jcloudsProviderRenames.properties` (for classic) or a config file `org.apache.brooklyn.jcloudsrename` (for karaf). To do this I've refactored the existing structure for Class Renames.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/drigodwin/brooklyn-server jclouds-renames
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/508.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 #508
----
commit b83247c364ef1d58be125c825d45c1e455bf5254
Author: Duncan Godwin <du...@cloudsoftcorp.com>
Date: 2017-01-06T14:24:19Z
Add jclouds rename provider & refactor
commit 48a92d5f15fef4ad2d665fff036e9516fda4e44f
Author: Duncan Godwin <du...@cloudsoftcorp.com>
Date: 2017-01-06T15:10:40Z
Added tests & refactoring
----
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r95156115
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingJcloudsRenamesProvider.java ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+import java.util.Arrays;
+
+public class DeserializingJcloudsRenamesProvider extends DeserializingProvider{
+
+ private static DeserializingJcloudsRenamesProvider instance;
+
+ public static DeserializingJcloudsRenamesProvider getInstance(){
+ if (instance == null) instance = new DeserializingJcloudsRenamesProvider();
+ return instance;
+ }
+
+ private DeserializingJcloudsRenamesProvider(){
+ super(Arrays.asList(new ConfigLoader[]{
--- End diff --
Ah, good point. Given that the super-constructor is protected, then that's ok. Could instead use: `super(MutableList.of(new JcloudsProviderRenameConfigLoader()));`
As a side-note, if the constructor were public, then it would be better to take a copy of the list (rather than relying on the caller passing in a mutable list).
If we do continue to rely on the constructor being called with a mutable list (rather than it taking a copy), then probably worth adding a javadoc comment about that parameter.
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94982693
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingJcloudsRenamesProvider.java ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+import java.util.Arrays;
+
+public class DeserializingJcloudsRenamesProvider extends DeserializingProvider{
+
+ private static DeserializingJcloudsRenamesProvider instance;
+
+ public static DeserializingJcloudsRenamesProvider getInstance(){
+ if (instance == null) instance = new DeserializingJcloudsRenamesProvider();
+ return instance;
+ }
+
+ private DeserializingJcloudsRenamesProvider(){
+ super(Arrays.asList(new ConfigLoader[]{
--- End diff --
Same for `DeserializingClassRenamesProvider`
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94981908
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingClassRenamesProvider.java ---
@@ -43,49 +37,30 @@
* See karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml
*/
@Beta
-public class DeserializingClassRenamesProvider {
- private static final Logger LOG = LoggerFactory.getLogger(DeserializingClassRenamesProvider.class);
+public class DeserializingClassRenamesProvider extends DeserializingProvider{
- private static final List<ConfigLoader> loaders = Lists.newCopyOnWriteArrayList();
- static {
- loaders.add(new ClasspathConfigLoader());
- }
-
- private static volatile Map<String, String> cache;
+ private static DeserializingClassRenamesProvider instance;
- public static List<ConfigLoader> getLoaders() {
- return loaders;
+ public static DeserializingClassRenamesProvider getInstance(){
+ if (instance == null) instance = new DeserializingClassRenamesProvider();
--- End diff --
Need to be careful about synchronization for singletons. See Bloch's Effective Java second edition, item 3.
I don't think we need to instantiate it lazily - we can do it as soon as this class is loaded. I'd change it to:
```
private static final DeserializingClassRenamesProvider INSTANCE = new DeserializingClassRenamesProvider();
```
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94983344
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/JcloudsProviderRenameConfigLoader.java ---
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+public class JcloudsProviderRenameConfigLoader extends PropertiesConfigLoader {
--- End diff --
As for ClasspathConfigLoader, I don't think this class is pulling its weight - I'd delete it.
---
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 #508: Alternate names for JcloudsProviders
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/508
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94982612
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingClassRenamesProvider.java ---
@@ -43,49 +37,30 @@
* See karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml
*/
@Beta
-public class DeserializingClassRenamesProvider {
- private static final Logger LOG = LoggerFactory.getLogger(DeserializingClassRenamesProvider.class);
+public class DeserializingClassRenamesProvider extends DeserializingProvider{
- private static final List<ConfigLoader> loaders = Lists.newCopyOnWriteArrayList();
- static {
- loaders.add(new ClasspathConfigLoader());
- }
-
- private static volatile Map<String, String> cache;
+ private static DeserializingClassRenamesProvider instance;
- public static List<ConfigLoader> getLoaders() {
- return loaders;
+ public static DeserializingClassRenamesProvider getInstance(){
+ if (instance == null) instance = new DeserializingClassRenamesProvider();
+ return instance;
}
- @Beta
- public static Map<String, String> loadDeserializingClassRenames() {
- synchronized (DeserializingClassRenamesProvider.class) {
- if (cache == null) {
- MutableMap.Builder<String, String> builder = MutableMap.<String, String>builder();
- for (ConfigLoader loader : loaders) {
- builder.putAll(loader.load());
- }
- cache = builder.build();
- LOG.info("Class-renames cache loaded, size {}", cache.size());
- }
- return cache;
- }
+ private DeserializingClassRenamesProvider(){
+ super(Arrays.asList(new ConfigLoader[]{
+ new ClasspathConfigLoader()
+ }));
}
/**
* Handles inner classes, where the outer class has been renamed. For example:
- *
+ *
* {@code findMappedName("com.example.MyFoo$MySub")} will return {@code com.example.renamed.MyFoo$MySub}, if
* the renamed contains {@code com.example.MyFoo: com.example.renamed.MyFoo}.
*/
@Beta
- public static String findMappedName(String name) {
- return Reflections.findMappedNameAndLog(DeserializingClassRenamesProvider.loadDeserializingClassRenames(), name);
+ public String findMappedName(String name) {
+ return Reflections.findMappedNameAndLog(DeserializingClassRenamesProvider.getInstance().loadDeserializingMapping(), name);
--- End diff --
This can just call `loadDeserializingMapping(...)` - it should be the same instance as is returned by `DeserializingClassRenamesProvider.getInstance()`.
---
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 #508: Alternate names for JcloudsProviders
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on the issue:
https://github.com/apache/brooklyn-server/pull/508
retest this please
---
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 #508: Alternate names for JcloudsProviders
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/508
@drigodwin can you kick off the jenkins build again please (e.g. by making a change to the commit id by running `git commit --ammend; git push -f`).
The jenkins test failure looks unrelated, so we should look at that in a separate PR:
```
2017-01-06 14:40:00,515 INFO TESTNG INVOKING: "Surefire test" - org.apache.brooklyn.core.entity.ApplicationLifecycleStateTest.testStartsThenChildFailsButWithQuorumCausesAppToSucceed()
2017-01-06 14:40:00,586 INFO No Camp-YAML parser registered for parsing catalog item DSL; skipping DSL-parsing
2017-01-06 14:40:00,604 WARN Setting TestEntityImpl{id=pr3k9o55xr} on-fire due to problems when expected running, up=false, not-up-indicators: {myIndicator=Simulate not-up of child}
2017-01-06 14:40:00,605 WARN Setting Application[jncpaf3w] on-fire due to problems when expected running, up=true, problems: {service-lifecycle-indicators-from-children-and-members=Required entity not healthy: TestEntityImpl{id=pr3k9o55xr}}
2017-01-06 14:40:00,616 INFO TESTNG FAILED: "Surefire test" - org.apache.brooklyn.core.entity.ApplicationLifecycleStateTest.testStartsThenChildFailsButWithQuorumCausesAppToSucceed() finished in 99 ms
java.lang.AssertionError: entity=Application[jncpaf3w]; attribute=Sensor: service.state (org.apache.brooklyn.core.entity.lifecycle.Lifecycle) expected [running] but found [on-fire]
at org.apache.brooklyn.test.Asserts.fail(Asserts.java:754)
at org.apache.brooklyn.test.Asserts.failNotEquals(Asserts.java:144)
at org.apache.brooklyn.test.Asserts.assertEquals(Asserts.java:466)
at org.apache.brooklyn.core.entity.EntityAsserts.assertAttributeEquals(EntityAsserts.java:58)
at org.apache.brooklyn.core.entity.EntityAsserts$4.run(EntityAsserts.java:126)
at org.apache.brooklyn.test.Asserts$RunnableAdapter.call(Asserts.java:1351)
at org.apache.brooklyn.test.Asserts.succeedsContinually(Asserts.java:1034)
at org.apache.brooklyn.test.Asserts.succeedsContinually(Asserts.java:1013)
at org.apache.brooklyn.core.entity.EntityAsserts.assertAttributeEqualsContinually(EntityAsserts.java:124)
at org.apache.brooklyn.core.entity.ApplicationLifecycleStateTest.assertHealthContinually(ApplicationLifecycleStateTest.java:196)
at org.apache.brooklyn.core.entity.ApplicationLifecycleStateTest.testStartsThenChildFailsButWithQuorumCausesAppToSucceed(ApplicationLifecycleStateTest.java:170)
```
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94983722
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/PropertiesConfigLoader.java ---
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import org.apache.brooklyn.util.core.ResourceUtils;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.stream.Streams;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Enumeration;
+import java.util.Map;
+import java.util.Properties;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class PropertiesConfigLoader implements ConfigLoader {
+
+ private static final Logger LOG = LoggerFactory.getLogger(PropertiesConfigLoader.class);
+ private String propertiesPath;
+
+ protected PropertiesConfigLoader(String propertiesPath){
+ this.propertiesPath = propertiesPath;
+ }
+
+ @Override
+ public Map<String, String> load() {
+ try {
+ InputStream resource = new ResourceUtils(DeserializingClassRenamesProvider.class).getResourceFromUrl(propertiesPath);
--- End diff --
I'd use `new ResourceUtils(PropertiesConfigLoader.class, ...)`. Otherwise it's just confusing what the relationship is between this and `DeserializingClassRenamesProvider`.
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94984944
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsProviderAndApiLoaderTest.java ---
@@ -68,6 +69,35 @@ public void testRegisteredProvider() throws Exception {
ProviderRegistry.unregisterProvider(provider);
}
}
+
+ @Test
+ public void testRenamedRegisteredProvider() throws Exception {
+ String id = "my-example-provider";
+ String renamedId = "my-example-provider-renamed";
--- End diff --
Personal preference for using "oldId" and "newId". When I first read the variable names, It thought that "renamedId" was going to be what the provider had been renamed *to*, rather than what it was renamed *from*.
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94983575
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingProvider.java ---
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+import com.google.common.annotations.Beta;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+import java.util.Map;
+
+/*
+ * This provider keeps a cache of the class-renames, which is lazily populated (see {@link #cache}.
+ * Calling {@link #reset()} will set this cache to null, causing it to be reloaded next time
+ * it is requested.
+ *
+ * Loading the cache involves iterating over the {@link #loaders}, returning the union of
+ * the results from {@link Loader#load()}.
+ *
+ * Initially, the only loader is the basic {@link ClasspathConfigLoader}.
+ *
+ * However, when running in karaf the {@link OsgiConfigLoader} will be instantiated and added.
+ * See karaf/init/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+ */
+@Beta
+public class DeserializingProvider {
+
+ private static final Logger LOG = LoggerFactory.getLogger(DeserializingProvider.class);
+
+ private List<ConfigLoader> loaders;
+
+ protected DeserializingProvider(List<ConfigLoader> loaders){
+ this.loaders = loaders;
+ }
+
+ private volatile Map<String, String> cache;
+
+ public List<ConfigLoader> getLoaders() {
+ return loaders;
+ }
+
+ @Beta
+ public Map<String, String> loadDeserializingMapping() {
+ synchronized (DeserializingProvider.class) {
--- End diff --
Does it need to synchronize on the class? Now that this is non-static, can we not just do `synchronized (this) {` (and same in `reset()` obviously).
---
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 #508: Alternate names for JcloudsProviders
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-server/pull/508
I get an ugly exception Brooklyn with this change:
```
2017-01-10 12:06:39,640 WARN Failed to load properties file from classpath://org/apache/brooklyn/core/mgmt/persist/jcloudsProviderRenames.properties (continuing)
org.apache.brooklyn.util.exceptions.PropagatedRuntimeException: Error getting resource 'classpath://org/apache/brooklyn/core/mgmt/persist/jcloudsProviderRenames.properties' for class org.apache.brooklyn.core.mgmt.persist.PropertiesConfigLoader
at org.apache.brooklyn.util.exceptions.Exceptions.propagate(Exceptions.java:165) ~[org.apache.brooklyn-brooklyn-utils-common-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
Caused by: java.io.IOException: Error accessing classpath://org/apache/brooklyn/core/mgmt/persist/jcloudsProviderRenames.properties: java.io.IOException: org/apache/brooklyn/core/mgmt/persist/jcloudsProviderRenames.properties not found on classpath
at org.apache.brooklyn.util.core.ResourceUtils.getResourceFromUrl(ResourceUtils.java:232) ~[org.apache.brooklyn-brooklyn-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
Caused by: java.io.IOException: org/apache/brooklyn/core/mgmt/persist/jcloudsProviderRenames.properties not found on classpath
at org.apache.brooklyn.util.core.ResourceUtils.getResourceViaClasspath(ResourceUtils.java:371) ~[org.apache.brooklyn-brooklyn-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
```
Should another file have been included in the pull request?
---
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 #508: Alternate names for JcloudsProviders
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on the issue:
https://github.com/apache/brooklyn-server/pull/508
@sjcorbett it is missing that file, I haven't seen that exception however. I've opened #511 to add it.
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94983260
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/ClasspathConfigLoader.java ---
@@ -18,55 +18,17 @@
*/
package org.apache.brooklyn.core.mgmt.persist;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.Enumeration;
-import java.util.Map;
-import java.util.Properties;
-
-import org.apache.brooklyn.util.core.ResourceUtils;
-import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.stream.Streams;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
-
/**
* Loads the class-renames from the configuration file on the classpath.
*
* @see {@link #DESERIALIZING_CLASS_RENAMES_PROPERTIES_PATH}
*/
-public class ClasspathConfigLoader implements ConfigLoader {
- private static final Logger LOG = LoggerFactory.getLogger(ClasspathConfigLoader.class);
- private static final String DESERIALIZING_CLASS_RENAMES_PROPERTIES_PATH = "classpath://org/apache/brooklyn/core/mgmt/persist/deserializingClassRenames.properties";
+public class ClasspathConfigLoader extends PropertiesConfigLoader {
--- End diff --
I don't think this class pulls its weight anymore. I'd delete it, and just have the thing that constructed it call `new PropertiesConfigLoader(DESERIALIZING_CLASS_RENAMES_PROPERTIES_PATH)`.
I might then rename `PropertiesConfigLoader` to indicate that it's loading the properties from the classpath.
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94984649
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsProviderAndApiLoader.java ---
@@ -84,13 +85,15 @@ public static boolean isApi(String id) {
}
public static Optional<ProviderMetadata> getProvider(String id) {
+ id = DeserializingJcloudsRenamesProvider.getInstance().applyJcloudsRenames(id);
--- End diff --
I wonder if we should overwrite the provider config key with the new name. Otherwise, we'll need to leave these values in the properties file forever. But I'm fine with us not doing that in this PR.
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94984349
--- Diff: karaf/init/src/main/java/org/apache/brooklyn/launcher/osgi/ClasspathOsgiConfigLoader.java ---
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.launcher.osgi;
+
+import org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+/**
+ * Loads the class-renames from the OSGi configuration file: {@code org.apache.brooklyn.classrename.cfg}.
+ *
+ * Only public for OSGi instantiation - treat as an internal class, which may change in
+ * future releases.
+ *
+ * See http://stackoverflow.com/questions/18844987/creating-a-blueprint-bean-from-an-inner-class:
+ * we unfortunately need to include {@code !org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider}
+ * in the Import-Package, as the mvn plugin gets confused due to the use of this inner class
+ * within the blueprint.xml.
+ *
+ * @see {@link #KARAF_DESERIALIZING_CLASS_RENAMES_PROPERTIES}
+ */
+public class ClasspathOsgiConfigLoader extends OsgiConfigLoader{
--- End diff --
I'd probably go for "ClassRenameOsgiConfigLoader"
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94983917
--- Diff: karaf/init/src/main/java/org/apache/brooklyn/launcher/osgi/ClasspathOsgiConfigLoader.java ---
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.launcher.osgi;
+
+import org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+/**
+ * Loads the class-renames from the OSGi configuration file: {@code org.apache.brooklyn.classrename.cfg}.
+ *
+ * Only public for OSGi instantiation - treat as an internal class, which may change in
+ * future releases.
+ *
+ * See http://stackoverflow.com/questions/18844987/creating-a-blueprint-bean-from-an-inner-class:
+ * we unfortunately need to include {@code !org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider}
+ * in the Import-Package, as the mvn plugin gets confused due to the use of this inner class
+ * within the blueprint.xml.
+ *
+ * @see {@link #KARAF_DESERIALIZING_CLASS_RENAMES_PROPERTIES}
+ */
+public class ClasspathOsgiConfigLoader extends OsgiConfigLoader{
--- End diff --
In what way is this "Classpath" OsgiConfigLoader?
---
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 #508: Alternate names for JcloudsProviders
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/508#discussion_r95144785
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingJcloudsRenamesProvider.java ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+import java.util.Arrays;
+
+public class DeserializingJcloudsRenamesProvider extends DeserializingProvider{
+
+ private static DeserializingJcloudsRenamesProvider instance;
+
+ public static DeserializingJcloudsRenamesProvider getInstance(){
+ if (instance == null) instance = new DeserializingJcloudsRenamesProvider();
+ return instance;
+ }
+
+ private DeserializingJcloudsRenamesProvider(){
+ super(Arrays.asList(new ConfigLoader[]{
--- End diff --
If I did this, as described [here](http://stackoverflow.com/questions/2776975/how-can-i-add-to-list-extends-number-data-structures), I wouldn't be able to add new elements to the list as is required by the `OsgiConfigLoader`.
---
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 #508: Alternate names for JcloudsProviders
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/508
Thanks @drigodwin - LGTM; merging.
Not sure why jenkins is still showing "validating", 20 hours later. The link is to https://builds.apache.org/job/brooklyn-server-pull-requests/1547, which was kicked off at 09-Jan-2017 15:17:14 and the console output shows "BUILD SUCCESS". Therefore I'm going to assume the build was fine and that it's a jenkins thing!
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94984056
--- Diff: karaf/init/src/main/java/org/apache/brooklyn/launcher/osgi/JcloudsProviderRenameOsgiConfigLoader.java ---
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.launcher.osgi;
+
+import org.apache.brooklyn.core.mgmt.persist.DeserializingJcloudsRenamesProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+public class JcloudsProviderRenameOsgiConfigLoader extends OsgiConfigLoader{
--- End diff --
Nitpick: personal preference for a space before the opening `{`
---
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 #508: Alternate names for JcloudsProviders
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/508#discussion_r94982414
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/persist/DeserializingJcloudsRenamesProvider.java ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.mgmt.persist;
+
+import java.util.Arrays;
+
+public class DeserializingJcloudsRenamesProvider extends DeserializingProvider{
+
+ private static DeserializingJcloudsRenamesProvider instance;
+
+ public static DeserializingJcloudsRenamesProvider getInstance(){
+ if (instance == null) instance = new DeserializingJcloudsRenamesProvider();
+ return instance;
+ }
+
+ private DeserializingJcloudsRenamesProvider(){
+ super(Arrays.asList(new ConfigLoader[]{
--- End diff --
Nit-picking... I'd the more concise: `super(ImmutableList.of(new JcloudsProviderRenameConfigLoader()));`
And I'd change the `DeserializingProvider` constructor to take `(List<? extends ConfigLoader> loaders)`.
---
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.
---