Catalog internal references

[ahgittin]

Add/update some test cases for internal references in catalog BOMs, one working, two failing but marked WIP so this is safe to merge

Working on fixing it as part of #746

[aledsage]

We've recommended in the docs (http://brooklyn.apache.org/v/0.11.0/ops/catalog/) that one should use services: with itemType: template, and not with itemType: entity (because the latter only ever has one thing in it).

Can you change this test to use itemType: template, or indicate in a comment at the top that this is testing a discouraged catalog format?

[aledsage]

As above, can we use itemType: template here if we're using services: ...?

[aledsage]

Wow, I didn't no we supported this. Why?!

It looks like classpath://yaml-ref-catalog.yaml contains:

name: Basic app
services:
- name: service
  type: yaml.basic:0.1.2

so it's not a catalog item. Should we really be able to reference it as a type in a catalog item like this? I'd have thought the right way of doing it (for an end-user) would be to re-write yaml-ref-catalog.yaml as a catalog item, deploy it to the catalog, and then reference its type in the normal way (which clearly would invalidate this test case).

I suggest we deprecate this asap. I don't think we've documented it anywhere, so hopefully no-one is relying on it in anger.

I therefore think it's not worth fixing this test.

[ahgittin]

good points -- i agree with @aledsage about discouraging this type of reference

i think what we want to support is:

• items: [ URL, ... ] where URL points at another brooklyn.catalog format YAML
• item: URL where URL points at an item YAML (non-catalog-format)

all other things should be deprecated?

(fixing the test will probably happen for free fixing the other test)

i will add an explicit test where it is a template and a note that the other format is deprecated (its behaviour is different so i will leave it)

[ahgittin]

@neykov does that make sense to you? you did much of this so want to make sure we aren't missing important things!

[neykov]

Support for the type: url was added around the same time we added support for yaml catalog items. For a long time it was the only way to compose yaml files out of other yaml files, using them as building blocks. It works at the blueprint level, unaware of catalog items. This only changed with the addition of the include keyword.
Don't think it ever got a lot of use. +1 to deprecate and remove.

• items: [ URL, ... ] - +1, it's the currently recommended way to compose catalogs
• item: URL - -1. What are your reasons to keep it @alx? I think we should deprecate this one as well. Have just a single way of including other items, through the catalog mechanism.

[ahgittin]

@aledsage tests updated with comments and explicitly testing template as well and item: URL syntax in addition to type: URL pointing at item yaml (not sure about test for items: [ URL ] pointing at catalog yaml, or even if we support that currently)

in any case to some extend feels like that should be a separate PR; this PR is just about testing references to co-bundled items, all of which should work if we have a sensible approach to adding items.

[ahgittin]

+1 to deprecating type: URL as it implies a deploy-time URL lookup which can change (at least on rebind, if we switch to persisting BOMs instead of CatalogItems maybe sooner)

reason for item: URL is it's the only way to use an item YAML file in the catalog. e.g. you've built a YAML which deploys just fine. you might want to save that as a file for easy editing and ad hoc re-deploy. but you also want to put it in a catalog. this lets you define the metadata in the catalog yaml and then reference the item yaml.

[neykov]

Sounds reasonable (on first read :) ). Still seems like a hacky workflow though. Separating the metadata and the content just so it's friendly to ad-hoc editing is not encouraging good dev practices.

[ahgittin]

Failure unrelated, due to infra: ERROR: ubuntu-2 is offline; cannot locate JDK 1.8 (latest)

Retest this please.

[ahgittin]

@neykov I've never liked the split between catalog metadata and item definition. Given that we have it currently, it doesn't feel like a bad dev practise to disallow their separation during dev.

If we could remove the split we could make the BOM YAML simpler and remove the need for the two paths. That would mean putting catalog metadata into the definition, IE baking fields such as id, description, and icon_url into the definitions -- and I guess being able to ignore them if given to the type instantiation and that ignores them. But that feels messier than having the split?

[ahgittin]

In any case a longer discussion than this PR which was only intended to test intra-bundle references.

@neykov could you confirm I've correctly interpreted your intention in repairing your test, IE the first commit 758b359 ? If so then I think this is good to merge. We can deprecate syntaxes and rationalize item v items in the future.

[neykov]

@alx I think the fix makes sense.