You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2014/11/10 18:45:24 UTC
[GitHub] incubator-brooklyn pull request: Versioning catalog items (take 2)
GitHub user neykov opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/312
Versioning catalog items (take 2)
Leaving here for early feedback.
Supersedes #92
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/incubator-brooklyn feature/catalog-versions2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/312.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 #312
----
commit f5c4d2855ed289210eb98e08a62725604189eb3a
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-07-17T15:37:42Z
REST API catalog versioning
commit fc019e58c8da791dd654f3b3f4eb33a0263cf8f4
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-11-05T16:45:55Z
Catalog items versioning - groundwork changes
commit 48f8f146dbe1c39ebab38af25e1157c358947c58
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-07-18T16:32:32Z
jsgui catalog versioning
commit 698f7d5396247af61c930f51a16f68516fee3ba2
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-07-21T16:08:02Z
Reference installed OSGi bundles by name+version
Also fix failing tests from previous commits.
commit 40499366d4a992b3ea39c47f348f44ab04621080
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-07-24T09:29:31Z
Using AssemblyTemplate.getName() for getting catalog items is wrong.
Catalog items are registered by id+version in the brooklyn.config section,
the name is unrelated which means that the items were never found.
The code path in BrooklynEntityMatcher is not needed anyway, already
handled by the instantiator.
commit 06a2c4dcc3359c39ec4620ae673c152b99f802ed
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-07-23T20:44:03Z
Fail on adding catalog item with same version.
Allow for override if forceUpdate is set.
commit 373fe74875275f953291c0ea87135cb6c67caf89
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-11-10T15:27:56Z
Clean up the CatalogItem interface
* override id to return getCatalogItemId()
* introduce symbolicName (analogous to old id/registeredTypeName)
* deprecate registeredTypeName in favor of symbolicName
* getCatalogItemId() should return symbolicName:version
* deprecate useless CatalogLibraries interface
* keep backwards compatibility for deserialization (registeredTypeName, libraries)
* remove CONGIGURATION catalog item type
Clean up the CatalogItemSymmary REST interface and udate JSGUI to match
* add symbolicName
* remove registeredTypeName
* keep id, but construct from other arguments
* keep type, but alias to symbolicName
commit be68dd105f5786fbe2a91e996a7b63cfa5553f10
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-11-10T16:56:39Z
Versioning - groundwork for returning the latest version if no version specified
commit f9faefb3af84b0f645bfb6e7b7948596698e032d
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-11-10T17:39:35Z
Versioning - when no explicit version use the highest version (preferring non-snapshot)
----
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128527
--- Diff: api/src/main/java/brooklyn/catalog/CatalogItem.java ---
@@ -34,12 +33,21 @@
public interface CatalogItem<T,SpecT> extends BrooklynObject, Rebindable {
public static enum CatalogItemType {
- TEMPLATE, ENTITY, POLICY, CONFIGURATION
+ TEMPLATE, ENTITY, POLICY
+ }
+
+ public static interface CatalogBundle {
+ public String getName();
--- End diff --
`SymbolicName` ?
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128585
--- Diff: core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java ---
@@ -387,38 +436,60 @@ public void load() {
Maybe<Object> possibleLibraries = catalog.getMaybe("libraries");
if (possibleLibraries.isAbsent()) possibleLibraries = catalog.getMaybe("brooklyn.libraries");
if (possibleLibraries.isPresentAndNonNull()) {
- if (!(possibleLibraries.get() instanceof List))
+ if (!(possibleLibraries.get() instanceof Collection))
throw new IllegalArgumentException("Libraries should be a list, not "+possibleLibraries.get());
- libraries = CatalogLibrariesDto.from((List<?>) possibleLibraries.get());
+ libraries = CatalogItemDtoAbstract.parseLibraries((Collection<?>) possibleLibraries.get());
}
- // TODO clear up the abundance of id, name, registered type, java type
- String registeredTypeName = (String) catalog.getMaybe("id").orNull();
- if (Strings.isBlank(registeredTypeName))
- registeredTypeName = (String) catalog.getMaybe("name").orNull();
+ String symbolicName;
+ String version = null;
+
+ symbolicName = (String) catalog.getMaybe("symbolicName").orNull();
+ if (Strings.isBlank(symbolicName)) {
+ symbolicName = (String) catalog.getMaybe("id").orNull();
+ if (Strings.isNonBlank(symbolicName) && CatalogUtils.looksLikeVersionedId(symbolicName)) {
+ symbolicName = CatalogUtils.getIdFromVersionedId(symbolicName);
+ version = CatalogUtils.getVersionFromVersionedId(symbolicName);
+ }
+ }
+ if (Strings.isBlank(symbolicName))
+ symbolicName = (String) catalog.getMaybe("name").orNull();
// take name from plan if not specified in brooklyn.catalog section not supplied
- if (Strings.isBlank(registeredTypeName)) {
- registeredTypeName = plan.getName();
- if (Strings.isBlank(registeredTypeName)) {
+ if (Strings.isBlank(symbolicName)) {
+ symbolicName = plan.getName();
+ if (Strings.isBlank(symbolicName)) {
if (plan.getServices().size()==1) {
Service svc = Iterables.getOnlyElement(plan.getServices());
- registeredTypeName = svc.getServiceType();
+ symbolicName = svc.getServiceType();
}
}
}
+ Maybe<Object> possibleVersion = catalog.getMaybe("version");
+ if (possibleVersion.isAbsent() && Strings.isBlank(version)) {
+ throw new IllegalArgumentException("'version' attribute missing in 'brooklyn.catalog' section.");
+ } else if (possibleVersion.isPresent()) {
+ if (Strings.isNonBlank(version)) {
+ throw new IllegalArgumentException("Can't use both attribute 'version' and versioned id");
--- End diff --
i'd be in favour of allowing if they match; someone might for some reason want `id` and `symbolicName` and `version` in their json (eg if you get full details from a server and just send that back)
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62883647
failure is `java.rmi.server.ExportException: Port already in use: 40125` -- suspect that is a random unrelated problem, but will investigate as part of review
---
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: Versioning catalog items (take 2)
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/312#discussion_r20288002
--- Diff: core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java ---
@@ -948,17 +954,54 @@ protected BrooklynMementoRawData loadMementoRawData(final RebindExceptionHandler
RebindTracker.reset();
}
}
-
+
+ private boolean forceCatalogItem(EntityMementoManifest entityManifest) {
--- End diff --
`shouldForceCatalogItem` ?
---
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: Versioning catalog items (take 2)
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#discussion_r20136740
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogXmlSerializer.java ---
@@ -54,7 +59,7 @@ public CatalogXmlSerializer() {
xstream.registerConverter(new EnumCaseForgivingSingleValueConverter(CatalogScanningModes.class));
xstream.aliasType("libraries", CatalogLibrariesDto.class);
- xstream.addImplicitCollection(CatalogLibrariesDto.class, "bundles", "bundle", String.class);
+ xstream.addImplicitCollection(CatalogLibrariesDto.class, "bundles", "bundle", CatalogBundleDto.class);
--- End diff --
It will be. I've put a note to myself to check if it can be done in a backwards compatible without introducing additional properties. Not sure if it's worth doing anything though because this concerns only catalog.xml deserialization which I haven't seen used with libraries so far.
I'll check if I can do anything about it and if not will ask for feedback on the mailing list.
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128819
--- Diff: core/src/main/java/brooklyn/management/ha/OsgiManager.java ---
@@ -137,24 +162,26 @@ public void registerBundle(String bundleUrl) {
}
return Maybe.of(clazz);
} else {
- bundleProblems.put(bundleUrlOrNameVersionString, new IllegalStateException("Unable to find bundle "+bundleUrlOrNameVersionString));
+ bundleProblems.put(catalogBundle, new IllegalStateException("Bundle is not installed"));
}
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
- if (noVersionInstalled) {
- if (bundleUrlOrNameVersionString.contains("/")) {
- // suppress misleading nested trace if the input string looked like a URL
- bundleProblems.put(bundleUrlOrNameVersionString, new IllegalStateException("Bundle does not appear to be installed"));
- } else {
- bundleProblems.put(bundleUrlOrNameVersionString, new IllegalStateException("Bundle does not appear to be installed", e));
- }
- } else {
- bundleProblems.put(bundleUrlOrNameVersionString, e);
- }
+//TODO Can't figure out the logic of the following branching.
--- End diff --
we discussed this but in any case i think i've cleaned it up
---
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: Versioning catalog items (take 2)
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/312#discussion_r20287927
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java ---
@@ -104,18 +107,35 @@ public String getIconUrl() {
}
@Override
- public String getVersion() {
- return version;
+ public String getSymbolicName() {
+ if (symbolicName != null) return symbolicName;
+ if (registeredTypeName != null) return registeredTypeName;
+ return getJavaType();
}
- @Nonnull
@Override
- public CatalogItemLibraries getLibraries() {
- return getLibrariesDto();
+ public String getVersion() {
+ //xstream doesn't call constructors
--- End diff --
makes sense -- worth an extra few words in the comment in case someone tries to clean it up 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: Versioning catalog items (take 2)
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/312#discussion_r20288087
--- Diff: core/src/main/java/brooklyn/entity/rebind/persister/CatalogItemLibrariesConverter.java ---
@@ -0,0 +1,64 @@
+/*
+ * 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 brooklyn.entity.rebind.persister;
+
+import java.util.ArrayList;
+import java.util.Collection;
+
+import brooklyn.catalog.CatalogItem.CatalogBundle;
+import brooklyn.catalog.CatalogItem.CatalogItemLibraries;
+import brooklyn.catalog.internal.CatalogBundleDto;
+
+import com.thoughtworks.xstream.converters.Converter;
+import com.thoughtworks.xstream.converters.MarshallingContext;
+import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import com.thoughtworks.xstream.io.HierarchicalStreamReader;
+import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
+
+@Deprecated
--- End diff --
explanation of deprecation
---
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: Versioning catalog items (take 2)
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/312#discussion_r20287898
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java ---
@@ -0,0 +1,50 @@
+/*
+ * 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 brooklyn.catalog.internal;
+
+import java.util.Comparator;
+
+import brooklyn.catalog.CatalogItem;
+import brooklyn.util.text.NaturalOrderComparator;
+
+public class CatalogItemComparator implements Comparator<CatalogItem<?, ?>> {
--- End diff --
javadoc handy here
---
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: Versioning catalog items (take 2)
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62881933
Depends on #325 to fix failing tests.
---
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: Versioning catalog items (take 2)
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#discussion_r20136332
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java ---
@@ -104,18 +107,35 @@ public String getIconUrl() {
}
@Override
- public String getVersion() {
- return version;
+ public String getSymbolicName() {
+ if (symbolicName != null) return symbolicName;
+ if (registeredTypeName != null) return registeredTypeName;
+ return getJavaType();
}
- @Nonnull
@Override
- public CatalogItemLibraries getLibraries() {
- return getLibrariesDto();
+ public String getVersion() {
+ //xstream doesn't call constructors
--- End diff --
The property is set to NO_VERSION when the object is initialized so it's not supposed to be null ever. But xstream doesn't call constructors when reading from the catalog.xml file which results in null value for the version property. That's why we have to fix it in the getter.
---
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: Versioning catalog items (take 2)
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/312#discussion_r20288106
--- Diff: core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java ---
@@ -72,6 +72,8 @@
* Defaults to false if system property is not set.
*/
public static final String FEATURE_RENAME_THREADS = "brooklyn.executionManager.renameThreads";
+
+ public static final String FEATURE_INFER_CATALOG_ITEM_ON_REBIND = "brooklyn.backwardCompatibility.feature.inferCatalogItemOnRebind";
--- End diff --
explanation
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62972319
looks like nice tidies -- will try with this
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128651
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogItemComparator.java ---
@@ -0,0 +1,33 @@
+package brooklyn.catalog.internal;
+
+import java.util.Comparator;
+
+import brooklyn.catalog.CatalogItem;
+import brooklyn.util.text.NaturalOrderComparator;
+
+public class CatalogItemComparator implements Comparator<CatalogItem<?, ?>> {
+ private static final String SNAPSHOT = "SNAPSHOT";
+ private static final Comparator<String> COMPARATOR = new NaturalOrderComparator();
--- End diff --
`NaturalOrderComparator.INSTANCE`
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62910269
the inferring catalog items doesn't work very well as classes usually have `*Impl` and so don't match; i guess we ought to do a full catalog scan.
apart from that and my commit however only minor comments.
suggest we merge now and address improving catalog scan subsequently
---
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: Versioning catalog items (take 2)
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/312
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62904814
unable to deserialize memento objects containing osgi-bundle items such as internal classes for effector bodies -- fixed in (personal branch commit) https://github.com/ahgittin/incubator-brooklyn/commit/2d1e27f2dfd778be6f18da014b434d0b730c1bde
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128723
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogXmlSerializer.java ---
@@ -54,7 +59,7 @@ public CatalogXmlSerializer() {
xstream.registerConverter(new EnumCaseForgivingSingleValueConverter(CatalogScanningModes.class));
xstream.aliasType("libraries", CatalogLibrariesDto.class);
- xstream.addImplicitCollection(CatalogLibrariesDto.class, "bundles", "bundle", String.class);
+ xstream.addImplicitCollection(CatalogLibrariesDto.class, "bundles", "bundle", CatalogBundleDto.class);
--- End diff --
is deserializing this going to be problematic?
---
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: Versioning catalog items (take 2)
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/312#discussion_r20298811
--- Diff: core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java ---
@@ -948,17 +954,54 @@ protected BrooklynMementoRawData loadMementoRawData(final RebindExceptionHandler
RebindTracker.reset();
}
}
-
+
+ private boolean forceCatalogItem(EntityMementoManifest entityManifest) {
--- End diff --
this goes away entirely after my commit for osgi-bundle classloader for memento deserialization
---
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: Versioning catalog items (take 2)
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/312#discussion_r20288055
--- Diff: core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java ---
@@ -948,17 +954,54 @@ protected BrooklynMementoRawData loadMementoRawData(final RebindExceptionHandler
RebindTracker.reset();
}
}
-
+
+ private boolean forceCatalogItem(EntityMementoManifest entityManifest) {
+ return entityManifest.getCatalogItemId() == null &&
+ BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND);
+ }
+
private String findCatalogItemId(Map<String, EntityMementoManifest> entityIdToManifest, EntityMementoManifest entityManifest) {
- EntityMementoManifest ptr = entityManifest;
- while (ptr != null) {
- if (ptr.getCatalogItemId() != null) {
- return ptr.getCatalogItemId();
+ if (entityManifest.getCatalogItemId() != null) {
+ return entityManifest.getCatalogItemId();
+ }
+
+ if (BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND)) {
+ //First check if any of the parent entities has a catalogItemId set.
+ EntityMementoManifest ptr = entityManifest;
+ while (ptr != null) {
+ if (ptr.getCatalogItemId() != null) {
+ CatalogItem<?, ?> catalogItem = CatalogUtils.getCatalogItemOptionalVersion(managementContext, ptr.getCatalogItemId());
+ if (catalogItem != null) {
+ return catalogItem.getId();
+ } else {
+ //Couldn't find a catalog item with this id, but return it anyway and
+ //let the caller deal with the error.
+ return ptr.getCatalogItemId();
+ }
+ }
+ if (ptr.getParent() != null) {
+ ptr = entityIdToManifest.get(ptr.getParent());
+ } else {
+ ptr = null;
+ }
}
- if (ptr.getParent() != null) {
- ptr = entityIdToManifest.get(ptr.getParent());
- } else {
- ptr = null;
+
+ //If no parent entity has the catalogItemId set try to match them by the type we are trying to load.
+ //The current convention is to set catalog item IDs to the java type (for both plain java or CAMP plan) they represent.
+ //This will be applicable only the first time the store is rebinded, while the catalog items don't have the default
+ //version appended to their IDs, but then we will have catalogItemId set on entities so not neede further anyways.
+ BrooklynCatalog catalog = managementContext.getCatalog();
+ ptr = entityManifest;
+ while (ptr != null) {
+ CatalogItem<?, ?> catalogItem = catalog.getCatalogItem(ptr.getType(), BrooklynCatalog.DEFAULT_VERSION);
--- End diff --
this feels fragile, assuming version-less and java type matching catalog symbolic name, rather than looking at catalog item's java type -- but maybe good enough to test with
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62977576
and it works perfectly. merging.
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128883
--- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java ---
@@ -116,8 +133,23 @@ public CatalogEntitySummary getEntity(
@PathParam("entityId") String entityId) throws Exception ;
@GET
+ @Path("/entities/{entityId}/{version}")
--- End diff --
interesting to use `version` in path -- i like
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128960
--- Diff: utils/common/src/main/java/brooklyn/util/io/FileUtil.java ---
@@ -197,4 +203,8 @@ private static int exec(List<String> cmds, OutputStream out, OutputStream err) {
Streams.closeQuietly(errgobbler);
}
}
+
+ public static String getSafeFileName(String str) {
--- End diff --
see `Strings.makeValidFilename(..)`. it whitelists chars and trims repeated `_`. makes sense to have a method here but one should refer to the other
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62979386
i'm just gonna stick a couple log messages into the find-in-catalog part
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128678
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java ---
@@ -32,55 +34,55 @@
import brooklyn.entity.rebind.BasicCatalogItemRebindSupport;
import brooklyn.entity.rebind.RebindSupport;
import brooklyn.mementos.CatalogItemMemento;
+import brooklyn.util.collections.MutableList;
import brooklyn.util.flags.FlagUtils;
import brooklyn.util.flags.SetFromFlag;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynObject implements CatalogItem<T, SpecT> {
- private static final Logger log = LoggerFactory.getLogger(CatalogItemDtoAbstract.class);
-
- // TODO are ID and registeredType the same?
- @SetFromFlag Set<Object> tags = Sets.newLinkedHashSet();
- @SetFromFlag String registeredType;
- @SetFromFlag String javaType;
- @SetFromFlag String description;
- @SetFromFlag String iconUrl;
- @SetFromFlag String version;
- @SetFromFlag CatalogLibrariesDto libraries;
- @SetFromFlag String planYaml;
-
- // Field left named `name' to maintain the name element in existing catalogues.
- @SetFromFlag("displayName") String name;
-
- /** @deprecated since 0.7.0.
- * used for backwards compatibility when deserializing.
- * when catalogs are converted to new yaml format, this can be removed. */
- @Deprecated
- @SetFromFlag
- String type;
+ private static Logger LOG = LoggerFactory.getLogger(CatalogItemDtoAbstract.class);
+
+ private @SetFromFlag String symbolicName;
+ private @SetFromFlag String version = BasicBrooklynCatalog.NO_VERSION;
+
+ /**@deprecated since 0.7.0, left for deserialization backwards compatibility */
+ private @Deprecated @SetFromFlag String registeredTypeName;
--- End diff --
great, keeping these with deprecation comments
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128691
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java ---
@@ -104,18 +107,35 @@ public String getIconUrl() {
}
@Override
- public String getVersion() {
- return version;
+ public String getSymbolicName() {
+ if (symbolicName != null) return symbolicName;
+ if (registeredTypeName != null) return registeredTypeName;
+ return getJavaType();
}
- @Nonnull
@Override
- public CatalogItemLibraries getLibraries() {
- return getLibrariesDto();
+ public String getVersion() {
+ //xstream doesn't call constructors
--- End diff --
don't understand this comment
---
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: Versioning catalog items (take 2)
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#issuecomment-62493222
cursory pass. really like 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] incubator-brooklyn pull request: Versioning catalog items (take 2)
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/312#discussion_r20136821
--- Diff: core/src/main/java/brooklyn/management/ha/OsgiManager.java ---
@@ -137,24 +162,26 @@ public void registerBundle(String bundleUrl) {
}
return Maybe.of(clazz);
} else {
- bundleProblems.put(bundleUrlOrNameVersionString, new IllegalStateException("Unable to find bundle "+bundleUrlOrNameVersionString));
+ bundleProblems.put(catalogBundle, new IllegalStateException("Bundle is not installed"));
}
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
- if (noVersionInstalled) {
- if (bundleUrlOrNameVersionString.contains("/")) {
- // suppress misleading nested trace if the input string looked like a URL
- bundleProblems.put(bundleUrlOrNameVersionString, new IllegalStateException("Bundle does not appear to be installed"));
- } else {
- bundleProblems.put(bundleUrlOrNameVersionString, new IllegalStateException("Bundle does not appear to be installed", e));
- }
- } else {
- bundleProblems.put(bundleUrlOrNameVersionString, e);
- }
+//TODO Can't figure out the logic of the following branching.
--- End diff --
Yes, forgot to update this. It is no longer needed because getRegisteredBundle doesn't throw.
---
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: Versioning catalog items (take 2)
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/312#discussion_r20287863
--- Diff: core/src/main/java/brooklyn/catalog/internal/CatalogBundleConverter.java ---
@@ -0,0 +1,59 @@
+/*
+ * 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 brooklyn.catalog.internal;
+
+import com.thoughtworks.xstream.converters.Converter;
+import com.thoughtworks.xstream.converters.MarshallingContext;
+import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import com.thoughtworks.xstream.converters.reflection.ReflectionConverter;
+import com.thoughtworks.xstream.converters.reflection.ReflectionProvider;
+import com.thoughtworks.xstream.io.HierarchicalStreamReader;
+import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+
+@Deprecated
--- End diff --
explanation
---
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: Versioning catalog items (take 2)
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/312#discussion_r20128803
--- Diff: core/src/main/java/brooklyn/management/ha/OsgiManager.java ---
@@ -49,9 +51,14 @@
/* see Osgis for info on starting framework etc */
+ private class VersionedName {
--- End diff --
@aledsage wanted this, i was being too lazy, returning `String[]` in places. glad you've done it. could be public i think.
you will have merge conflicts since #307 unfortunately. hopefully easy.
---
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.
---