You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/09/16 09:36:42 UTC

[GitHub] brooklyn-library pull request #128: Add icons to catalog.bom files

GitHub user aledsage opened a pull request:

    https://github.com/apache/brooklyn-library/pull/128

    Add icons to catalog.bom files

    I've deliberately only focused on the .bom files in each module. I've not touched `karaf/catalog/src/main/resources/library-catalog-classes.bom`. At some point soon'ish, we should look at removing that duplication so it gets the .bom files from the bundles, rather than from a separate single .bom file. As such, I didn't want to spend my time updating the file that we'll likely subsequently want to delete!

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

    $ git pull https://github.com/aledsage/brooklyn-library add-catalogBom-icons

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

    https://github.com/apache/brooklyn-library/pull/128.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 #128
    
----
commit 45d22be2ab96a985c4222b2fa3432e8e0fad5832
Author: Aled Sage <al...@gmail.com>
Date:   2017-09-15T19:32:42Z

    Add icons to catalog.bom files

----


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    Massive +1 @aledsage, thanks for this, LGTM.
    
    Just one question though: Is the icon applied on the running instance if `iconUrl` is on the catalog item level (beside the catalog `id` like you did rather than down to one level, i.e within `item`) ?


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    retest this please


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    @tbouron yes, the rest endpoint `/children` also includes the `iconUrl` (for both the `pr-128-top-level` and `pr-128-inside-item`). I deployed the app shown below:
    ```
    services:
    - type: org.apache.brooklyn.entity.stock.BasicApplication
      brooklyn.children:
      - type: pr-128-top-level
    ```
    and then queried (via curl) for the children of the basic app.


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    Great news! Then it's +1 from me :)


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    @aledsage No I wouldn't. Having the`iconUrl` field returned when querying `http://localhost:8081/v1/applications/zlhibxdxxz/entities/zlhibxdxxz` is what we want.
    
    Thanks for the tests BTW, LGTM 👍  


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    Thanks @tbouron. I think the icon url needs to be at the top-level to be known about by the catalog, and that should be sufficient for it to be known by the entity instances.
    
    I added two catalog items:
    ```
    brooklyn.catalog:
        version: "1.0.1"
        itemType: entity
        items:
        - id: pr-128-top-level
          iconUrl: classpath:///ansible-logo.png
          name: PR 128 Icon at top
          item:
            type: org.apache.brooklyn.entity.stock.BasicApplication
    ```
    And:
    ```
    brooklyn.catalog:
        version: "1.0.1"
        itemType: entity
        items:
        - id: pr-128-inside-item
          name: PR 128 Icon inside item
          item:
            type: org.apache.brooklyn.entity.stock.BasicApplication
            iconUrl: classpath:///ansible-logo.png
    ```
    I queried the catalog for these (e.g. with `curl -v -u xxxx:xxxx http://localhost:8081/v1/catalog/entities/pr-128-top-level/1.0.1`).
    
    The response for the top-level .bom included: `"iconUrl":"/v1/catalog/icon/pr-128-top-level/1.0.1"`, but the inside-item .bom didn't (in the "planYaml" section it did show the iconUrl though).
    
    ---
    I deployed two apps:
    ```
    services:
    - type: pr-128-top-level
    ```
    and:
    ```
    services:
    - type: pr-128-inside-item
    ```
    I then queried the rest api about each entity (e.g. with `curl -v -u xxxx:xxxx http://localhost:8081/v1/applications/zlhibxdxxz/entities/zlhibxdxxz`).
    
    The app response for "top-level" included `"iconUrl":"/v1/applications/zlhibxdxxz/entities/zlhibxdxxz/icon"`, and the "inside-item" app had `"iconUrl":"/v1/applications/jyda9wjmx0/entities/jyda9wjmx0/icon"`. I downloaded both of those icons from the server. I confirmed that both were the icon defined in the original .bom files.
    
    ---
    @tbouron would you expect the icon to be defined/referenced anywhere else for the running instance? Anything else I should check?


---

[GitHub] brooklyn-library issue #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128
  
    @aledsage I forgot but does the endpoint `http://localhost:8081/v1/applications/zlhibxdxxz/entities/zlhibxdxxz/children` also returns the `iconUrl`?


---

[GitHub] brooklyn-library pull request #128: Add icons to catalog.bom files

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

    https://github.com/apache/brooklyn-library/pull/128


---