[GitHub] brooklyn-server pull request: Support removal of catalog entries f...

[geomacy]

Github user geomacy commented on the pull request:

https://github.com/apache/brooklyn-server/pull/100#issuecomment-208239512
  
hi @neykov that's squashed with previous commit and pushed in 47421cb



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Support removal of catalog entries f...

[neykov]

Github user neykov commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/100#discussion_r58990111
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java
 ---
@@ -100,54 +103,75 @@ public ManagementContext getManagementContext() {
 return managementContext;
 }
 
+/**
+ * Scans the bundle being added for a catalog.bom file and adds 
any entries in it to the catalog.
+ *
+ * @param bundle The bundle being added to the bundle context.
+ * @param bundleEvent The event of the addition.
+ *
+ * @return The items added to the catalog; these will be tracked 
by the {@link BundleTracker} mechanism
+ * and supplied to the {@link #removedBundle(Bundle, 
BundleEvent, Iterable)} method.
+ */
 @Override
-public Long addingBundle(Bundle bundle, BundleEvent bundleEvent) {
-
-final BundleContext bundleContext = 
FrameworkUtil.getBundle(CatalogBomScanner.class).getBundleContext();
-if (bundleContext == null) {
-LOG.info("Bundle context not yet established for bundle {} 
{} {}", bundleIds(bundle));
-return null;
-}
-
-scanForCatalog(bundle);
-
-return bundle.getBundleId();
+public Iterable> addingBundle(Bundle 
bundle, BundleEvent bundleEvent) {
+return scanForCatalog(bundle);
 }
 
-@Override
-public void modifiedBundle(Bundle bundle, BundleEvent event, Long 
bundleId) {
-sanityCheck(bundle, bundleId);
-LOG.info("Modified bundle {} {} {}", bundleIds(bundle));
-}
 
 @Override
-public void removedBundle(Bundle bundle, BundleEvent bundleEvent, 
Long bundleId) {
-sanityCheck(bundle, bundleId);
-LOG.info("Unloading catalog BOM from {} {} {}", 
bundleIds(bundle));
+public void removedBundle(Bundle bundle, BundleEvent bundleEvent, 
Iterable> items) {
+if (!items.iterator().hasNext()) {
+return;
+}
+LOG.debug("Unloading catalog BOM entries from {} {} {}", 
bundleIds(bundle));
+final BrooklynCatalog catalog = 
getManagementContext().getCatalog();
+for (CatalogItem item : items) {
+LOG.debug("Unloading {} {} from catalog", 
item.getSymbolicName(), item.getVersion());
+
+try {
+catalog.deleteCatalogItem(item.getSymbolicName(), 
item.getVersion());
+} catch (Exception e) {
+// Gobble exception to avoid possibility of causing 
problems stopping bundle.
--- End diff --

add `Exceptions.propagateIfFatal(e)` for the case of `InterruptedException`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Support removal of catalog entries f...

[geomacy]

Github user geomacy commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/100#discussion_r58729416
  
--- Diff: 
core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBomScanner.java
 ---
@@ -100,54 +103,82 @@ public ManagementContext getManagementContext() {
 return managementContext;
 }
 
+/**
+ * Scans the bundle being added for a catalog.bom file and adds 
any entries in it to the catalog.
+ *
+ * @param bundle The bundle being added to the bundle context.
+ * @param bundleEvent The event of the addition.
+ *
+ * @return The items added to the catalog; these will be tracked 
by the {@link BundleTracker} mechanism
+ * and supplied to the {@link #removedBundle(Bundle, 
BundleEvent, Iterable)} method.
+ */
 @Override
-public Long addingBundle(Bundle bundle, BundleEvent bundleEvent) {
+public Iterable> addingBundle(Bundle 
bundle, BundleEvent bundleEvent) {
+return scanForCatalog(bundle);
+}
 
-final BundleContext bundleContext = 
FrameworkUtil.getBundle(CatalogBomScanner.class).getBundleContext();
-if (bundleContext == null) {
-LOG.info("Bundle context not yet established for bundle {} 
{} {}", bundleIds(bundle));
-return null;
-}
 
-scanForCatalog(bundle);
+@Override
+public void removedBundle(Bundle bundle, BundleEvent bundleEvent, 
Iterable> items) {
+if (!items.iterator().hasNext()) {
+return;
+}
+LOG.debug("Unloading catalog BOM entries from {} {} {}", 
bundleIds(bundle));
+List exceptions = MutableList.of();
+final BrooklynCatalog catalog = 
getManagementContext().getCatalog();
+for (CatalogItem item : items) {
+LOG.debug("Unloading {} {} from catalog", 
item.getSymbolicName(), item.getVersion());
+
+try {
+catalog.deleteCatalogItem(item.getSymbolicName(), 
item.getVersion());
+} catch (Exception e) {
+LOG.warn("Caught {} unloading {} {} from catalog", new 
String [] {
+e.getMessage(), item.getSymbolicName(), 
item.getVersion()
+});
+exceptions.add(e);
+}
+}
 
-return bundle.getBundleId();
+if (0 < exceptions.size()) {
+throw new CompoundRuntimeException(
--- End diff --

Hm, I guess I have been assuming that this will just result in a message 
being logged by Karaf as opposed to it causing a failure - I will have to test 
that out.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request: Support removal of catalog entries f...

[geomacy]

GitHub user geomacy opened a pull request:

https://github.com/apache/brooklyn-server/pull/100

Support removal of catalog entries from bundle BOM when the bundle is 
stopped.


This change follows on from 
https://github.com/apache/brooklyn-server/pull/80.

With this change any catalog members that were added by scanning the 
bundle's
catalog.bom when the bundle started will be removed upon stopping the 
bundle.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/geomacy/brooklyn-server 
prune-catalog-on-bundle-stop

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/brooklyn-server/pull/100.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 #100


commit 7a1d51c6c9d0ea084ad09be51a27dec06fb0d365
Author: Geoff Macartney 
Date:   2016-04-05T13:22:05Z

Support removal of catalog entries from bundle BOM when the bundle is 
stopped.

This change follows on from 
https://github.com/apache/brooklyn-server/pull/80.

With this change any catalog members that were added by scanning the 
bundle's
catalog.bom when the bundle started will be removed upon stopping the 
bundle.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---