Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add icons to catalog.bom files #128

Merged
merged 1 commit into from
Sep 19, 2017
Merged

Conversation

aledsage
Copy link
Contributor

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!

@tbouron
Copy link
Member

tbouron commented Sep 16, 2017

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) ?

@aledsage
Copy link
Contributor Author

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?

@tbouron
Copy link
Member

tbouron commented Sep 18, 2017

@aledsage No I wouldn't. Having theiconUrl field returned when querying http://localhost:8081/v1/applications/zlhibxdxxz/entities/zlhibxdxxz is what we want.

Thanks for the tests BTW, LGTM 👍

@tbouron
Copy link
Member

tbouron commented Sep 18, 2017

@aledsage I forgot but does the endpoint http://localhost:8081/v1/applications/zlhibxdxxz/entities/zlhibxdxxz/children also returns the iconUrl?

@aledsage
Copy link
Contributor Author

@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.

@aledsage
Copy link
Contributor Author

retest this please

@tbouron
Copy link
Member

tbouron commented Sep 19, 2017

Great news! Then it's +1 from me :)

@asfgit asfgit merged commit 45d22be into apache:master Sep 19, 2017
asfgit pushed a commit that referenced this pull request Sep 19, 2017
@aledsage aledsage deleted the add-catalogBom-icons branch April 13, 2018 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants