You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2017/04/14 19:19:52 UTC

[GitHub] brooklyn-server pull request #638: Add support for managing enrichers within...

GitHub user tbouron opened a pull request:

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

    Add support for managing enrichers within the catalog

    This allows enrichers to be registered within the Brooklyn catalog, same as entities or policies. It allows enrichers' registration via:
    - `@Catalog` annotation on the java class
    - YAML with an `itemType: enricher`
    
    This also adds 3 endpoints to the `/v1/catalog` REST API:
    - `GET /v1/catalog/enrichers` returns the list of all registered enrichers
    - `GET /v1/catalog/enrichers/<enricherId>/<version>` returns the detail of a specific enricher
    - `DELETE /v1/catalog/enrichers/<enricherId>/<version>` details a specific enricher
    
    _PS: If the PR is too big, I can split up to multiples PRs. Hopefully the commit's messages are pretty clear_

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

    $ git pull https://github.com/tbouron/brooklyn-server feature/catalog-enrichers

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

    https://github.com/apache/brooklyn-server/pull/638.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 #638
    
----
commit 26c72b97773942bcdde64eb829cf860f7c3d43ce
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date:   2017-04-14T19:07:35Z

    Add support for managing enrichers within the catalog (automatically scanned and YAML parsed, same as entities or policies)

commit 17ef9ed360872a78c6d8b9094558ce9aa49b362f
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date:   2017-04-14T19:08:20Z

    Add @Catalog annotation to existing enrichers

commit 92ee24e5844e74975034a5f94bedd642d6012a3b
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date:   2017-04-14T19:09:16Z

    Add support for enrichers catalog items on the REST API (CRUD)

----


---
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 #638: Add support for managing enrichers within the ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    Thanks for the review @geomacy! I addressed your comments and rebased on the latest master


---
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 #638: Add support for managing enrichers within...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 #638: Add support for managing enrichers within...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/638#discussion_r114989208
  
    --- Diff: policy/src/main/java/org/apache/brooklyn/policy/ha/ServiceFailureDetector.java ---
    @@ -55,7 +56,7 @@
      * (or until another process manually sets {@link Attributes#SERVICE_STATE_ACTUAL} to {@value Lifecycle#ON_FIRE},
      * which this enricher will not clear until all problems have gone away)
      */
    -//@Catalog(name="Service Failure Detector", description="HA policy for deteting failure of a service")
    +@Catalog(name="Service Failure Detector", description="Emits a new sensor if the current entity has an issue")
    --- End diff --
    
    "has an issue" seems too general, would simply `if the current entity fails` be better?


---
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 #638: Add support for managing enrichers within the ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    @geomacy done!


---
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 #638: Add support for managing enrichers within the ca...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    @tbouron see brooklyn-server/policy/src/main/resources/catalog.bom - I think we'd need to list the enrichers in a .bom file, otherwise we won't list them when running in karaf (because we won't classpath-scan for annotated enrichers across all bundles, I believe).


---
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 #638: Add support for managing enrichers within the ca...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    Actually can you squash the commits?


---
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 #638: Add support for managing enrichers within...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/638#discussion_r114989012
  
    --- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java ---
    @@ -116,6 +120,8 @@ public static CatalogItemSummary catalogItemSummary(BrooklynRestResourceUtils b,
                     return catalogEntitySummary(b, item, ub);
                 case POLICY:
                     return catalogPolicySummary(b, item, ub);
    +                case ENRICHER:
    --- End diff --
    
    Indentation


---
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 #638: Add support for managing enrichers within the ca...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    Thanks, 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 pull request #638: Add support for managing enrichers within...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/638#discussion_r114989953
  
    --- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java ---
    @@ -144,6 +146,38 @@ static AssemblyTemplate resolveDeploymentPlan(String plan, BrooklynClassLoadingC
             return spec;
         }
     
    +    static EnricherSpec<?> createEnricherSpec(String yamlPlan, BrooklynClassLoadingContext loader, Set<String> encounteredCatalogTypes) {
    +        DeploymentPlan plan = makePlanFromYaml(loader.getManagementContext(), yamlPlan);
    +
    +        //Would ideally re-use the PolicySpecResolver
    --- End diff --
    
    `PolicySpecResolver` in the comment isn't right


---
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 #638: Add support for managing enrichers within the ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    @aledsage Tried to add enricher definitions in the `/src/main/resources/catalog.bom` of each bundle where the enricher is located, plus in `karaf/init/src/main/resources/catalog-classes.bom` but so far, they are not registered when I start Brooklyn in Karaf mode.
    
    Am I doing something wrong?


---
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 #638: Add support for managing enrichers within...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/638#discussion_r114989902
  
    --- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampInternalUtils.java ---
    @@ -144,6 +146,38 @@ static AssemblyTemplate resolveDeploymentPlan(String plan, BrooklynClassLoadingC
             return spec;
         }
     
    +    static EnricherSpec<?> createEnricherSpec(String yamlPlan, BrooklynClassLoadingContext loader, Set<String> encounteredCatalogTypes) {
    +        DeploymentPlan plan = makePlanFromYaml(loader.getManagementContext(), yamlPlan);
    +
    +        //Would ideally re-use the PolicySpecResolver
    +        //but it is CAMP specific and there is no easy way to get hold of it.
    +        Object policies = checkNotNull(plan.getCustomAttributes().get(BasicBrooklynCatalog.ENRICHERS_KEY), "enricher config");
    --- End diff --
    
    variable name is still `policies`


---
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 #638: Add support for managing enrichers within the ca...

Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:

    https://github.com/apache/brooklyn-server/pull/638
  
    Thanks @tbouron, that's great, 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.
---