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/12/10 16:02:30 UTC
[GitHub] incubator-brooklyn pull request: Improve recursion checks for cata...
GitHub user neykov opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/383
Improve recursion checks for catalog items
* `catalog.createSpec` now passes the symbolicName of the item so resolving to items with the same symbolicName already in the catalogue is prevented (fallback to java types only).
* the recursion check was using the type as is, thus making it possible to recurse into items with the same symbolicName where version is explicitly specified. Explicitly forbid catalog items referencing same symbolicName items, even if with another version.
* stop catalog items whose symbolicName matches the java type they export leaking their bundles to callers.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/incubator-brooklyn fix/catalog-recursive-resolve
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/383.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 #383
----
commit 46a242c397719d72a55a4ea43fcd5b87b7b0a5e9
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-12-10T13:50:39Z
Don't recurse into same symbolicName catalog items when resolving CAMP dependencies.
commit 5b3a75913dbe4167d95e95fc3907b4fb70412c22
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-12-10T14:56:17Z
Stop OSGi catalog items leaking libraries to parent.
The loader of the catalog item matching the type was used to load the type itself, while only the loader passed from caller should be used. If the catalog items' symbolicName was matching the java type they expose, they would be leaking their OSGi bundles.
----
---
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: Improve recursion checks for cata...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/383#issuecomment-71099474
@neykov Can you resolve the merge conflicts?
---
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: Improve recursion checks for cata...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/383#issuecomment-73231515
okay - i think that makes sense. thanks for the detailed 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: Improve recursion checks for cata...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/383#issuecomment-72200464
Presently all instantiations of CAMP YAML are done at the AssemblyTemplate level, no? Even "Add Child" creates an app and then strips it to get the entity. The same `encounteredCatalogTypes` set is used for the all components in the YAML so it shouldn't matter how deep a recursive reference is. As for instantiating entitites within Java code, then this is not applicable as the fix is for breaking catalog item cycles. It's highly probable that I dind't fully understand your comments, so feel free to correct my missunderstanding :).
The change improves how standalone catalog items are instantiated. This only applies when creating an `EntitySpec` from the catalog items directly (when adding them to the catalog to check the app type, when listing the catalog items from REST). When instantiating catalog items as part of an application template the `encounteredCatalogTypes` is already properly initialized.
Creating a spec out of a catalog item directly was treating the catalog yaml as an application, starting any exclusion in `encounteredCatalogTypes` one level below them. If the user was creating catalog items with `id` matching the `javaType` and there was already a catalog item with the same `symbolicName` in the catalog (i.e. when updating catalog items) then we would get the following structure
* new catalog item
* old catalog item
* java type resolved from old catalog item
With the change in place we get:
* new catalog item
* java type resolved from new catalog item
The difference is that now we initialize `encounteredCatalogTypes` in `BasicBrooklynCatalog` forcing the code to ignore existing catalog items with the same `id`.
---
Re `tryLoadEntityClass`. With the following in the catalog:
```
brooklyn.catalog:
id: sample.java.Type
libraries:
- http://simple.jar
services:
- type: sample.java.Type
```
and then updaing the catalog with
```
brooklyn.catalog:
id: sample.java.Type
libraries:
- http://invalid-url-or-libraries-missing.jar
services:
- type: sample.java.Type
```
Where the second catalog item doesn't have any `libraries` or they don't have `sample.java.Type` in them - resolving `sample.java.Type` in the second catalog item would be done with the `OsgiBrooklynClassLoadingContext` of the first item, so it would successfully resolve the java class.
---
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: Improve recursion checks for cata...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/383#issuecomment-71853205
@sjcorbett Rebased on latest master, conflicts are resolved.
---
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: Improve recursion checks for cata...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/383
---
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.
---