You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/02/28 12:59:29 UTC
[GitHub] brooklyn-server pull request #573: Do not runtime-inherit catalog item
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/573
Do not runtime-inherit catalog item
fixes https://issues.apache.org/jira/browse/BROOKLYN-445 and failing tests included here
means catalog BOMs should be written such that any entity/policy/etc which wants to access resources in the bundle is defined as its own item
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server do-not-inherit-catalog-item-id
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/573.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 #573
----
commit d7cbaeed46d7fedf8e49df015baa812352060f45
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-02-28T12:43:14Z
add original failing test from @sjcorbett
for failure to set inherited config keys - https://issues.apache.org/jira/browse/BROOKLYN-445
commit edaf05441ee46d05e4479a79aee055e6cbfe1237
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-02-28T12:44:17Z
add similar tests, including one failing, isolating the error
commit 9c38c94eeeb5a3de2a082f81546440980b1abb5c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-02-28T12:55:24Z
apply the fix for previous failing tests and https://issues.apache.org/jira/browse/BROOKLYN-445
do not use a context catalog item id. this is a significant change and release notes should be updated,
but it doesn't seem to break any of our tests; presumably most catalog items are already refactoring
so each item is declared as its own item.
----
---
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 #573: Do not runtime-inherit catalog item in the spec ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
@neykov @geomacy Think I've tidied the last bits. Could this be merged? Then we can focus on #338 getting in.
---
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 #573: Do not runtime-inherit catalog item in th...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/573
---
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 #573: Do not runtime-inherit catalog item in the spec ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
The changes include unrelated change in the class loading fallbacks - increasing the priority of the bundle prefix in relation to the other fallbacks. Contrary to the comment in https://github.com/apache/brooklyn-server/pull/573/commits/40330602af958d1e3486feea81933168d44e936d, the changes include behaviour changes, not only logging improvements. Would've been useful to have it in a separate commit + PR for better visibility if we ever need to come back to the changes.
The `RedisClusterIntegrationTest` would've been better added to the `brooklyn-camp` project which is the place to add CAMP tests which don't fit elsewhere - we really need to split them in an camp-integration-test project.
Comments above are not worth blocking the merge. Merging now.
---
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 #573: Do not runtime-inherit catalog item
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
@neykov correct. I will add this as a failing test and suggest that this PR:
https://github.com/apache/brooklyn-server/pull/338
seek to confirm that the test passes.
(marking this WIP until i fix)
---
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 #573: Do not runtime-inherit catalog item
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/573#discussion_r103960527
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java ---
@@ -792,6 +796,85 @@ public void testConfigParameterWithEntitySpecAsDefault() throws Exception {
}
@Test
+ public void testManuallyAddSpec() throws Exception {
+ String yaml = Joiner.on("\n").join(
+ "services:",
+ "- type: "+TestEntity.class.getName());
+
+ Entity app = createStartWaitAndLogApplication(yaml);
+ TestEntity entity1 = (TestEntity) Iterables.getOnlyElement(app.getChildren());
+
+ TestEntity entity2 = entity1.addChild(EntitySpec.create(TestEntity.class));
+ entity2.start(Collections.<Location>emptyList());
+
+ Entities.dumpInfo(app);
+
+ LOG.info("E1 keys: "+entity1.getEntityType().getConfigKeys());
+ LOG.info("E2 keys: "+entity2.getEntityType().getConfigKeys());
+ Assert.assertEquals(entity2.getEntityType().getConfigKeys(), entity1.getEntityType().getConfigKeys());
+ }
+
+ @Test
+ public void testManuallyAddSpecFromCatalog() throws Exception {
+ addCatalogItems(
+ "brooklyn.catalog:",
+ " itemType: entity",
+ " items:",
+ " - id: test-entity",
+ " item:",
+ " type: "+TestEntity.class.getName());
+
+ String yaml = Joiner.on("\n").join(
+ "services:",
+ "- type: test-entity");
+
+ Entity app = createStartWaitAndLogApplication(yaml);
+ TestEntity entity1 = (TestEntity) Iterables.getOnlyElement(app.getChildren());
+
+ TestEntity entity2 = entity1.addChild(EntitySpec.create(TestEntity.class));
+ entity2.start(Collections.<Location>emptyList());
+
+ Entities.dumpInfo(app);
+
+ LOG.info("E1 keys: "+entity1.getEntityType().getConfigKeys());
+ LOG.info("E2 keys: "+entity2.getEntityType().getConfigKeys());
+ Assert.assertEquals(entity2.getEntityType().getConfigKeys(), entity1.getEntityType().getConfigKeys());
--- End diff --
Can you assert the `catalogItemId` of the entities as well.
---
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 #573: Do not runtime-inherit catalog item in the spec ...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
Regarding `RedisClusterIntegrationTest`, it specifically only failed when run through the http server. The same blueprint passed when tested with `AbstractYamlTest`.
---
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 #573: Do not runtime-inherit catalog item in the spec ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
Quite right re https://github.com/apache/brooklyn-server/pull/573/commits/40330602af958d1e3486feea81933168d44e936d -- hit a weird bug in testing related to this but missed it when making the PRs.
Agree re `Redis` except for the point Sam mentions. Good to have a few tests that go through the server.
---
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 #573: Do not runtime-inherit catalog item
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
Changes look good.
Do I correctly understand that the following is no longer possible?
```
brooklyn.catalog:
brooklyn.libraries:
...
items:
- id: test
version: 0.0.1-SNAPSHOT
item:
type: Application
brooklyn.children:
- type: VanillaSoftwareProcess
brooklyn.config:
files.install:
classpath://something-in-libraries: uploaded-script.sh
```
Or is it some other case that changes behaviour?
---
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 #573: [WIP] Do not runtime-inherit catalog item
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/573
@neykov Prescient suggestions. Completely forget there were _two_ other paths where `catalogItemId` was being set to an inheritance/context value.
It doesn't cause problems with config keys but it is messy and should be fixed. Natural place to do it is in #338 -- have added several more tests and TODO items here which should be looked at as part of that.
---
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.
---