Alex,
I say we break it - force the user to have the correct mental model for
1.0.0!
It's a simple one-line addition to fix a given .bom file. We should
obviously ensure that historic persisted state continues to work.
---
Also note that our deprecation warnings are too hidden. For example, if
you do `br catalog add ...` then deprecation warnings will only be
visible via the Brooklyn log (rather than being returned to the user of
`br` or via the REST api).
But that's a separate topic!
Aled
On 26/10/2017 12:58, Alex Heneveld wrote:
Absolutely.
Question is whether we should deprecate or make it a breaking change.
Deprecating feels right, though I think it would mean we can't
actually remove until 2.0 ?
Best
Alex
On 26/10/2017 12:06, Aled Sage wrote:
Hi all,
I propose we make it mandatory to specify the bundle id and version
(currently it is optional).
I think we should do this asap, before 1.0.0 is released.
This is a breaking change - it will require users to add a `bundle:
...` line to their .bom files (if they are not supplying the metadata
another way, such as building their project as an OSGi bundle).
TL;DR: for backwards compatibility, we let users have a different
mental model from what Brooklyn now actually does, which can lead to
confusing behaviour when upgrading or working with snapshots.
*## Current Behaviour*
Currently, we'll auto-wrap things as a bundle, generating a unique
anonymous bundle name:version if one is not supplied.
This is important for users doing `br catalog add myfile.bom` or `br
catalog add mydir/`. In both cases we automatically create an OSGi
bundle with those contents. For that bundle's name:version, one can
explicitly supply it (e.g. via `bundle: ...` in the .bom file). But,
for backwards compatibility, we support .bom files that have no such
metadata.
*## Old Behaviour (0.11.0 and Earlier)*
Before we added full support and use of bundles in the catalog, the
user's .bom file was parsed and its items added to the catalog. The
raw .bom file was discarded.
For upgrade/dependencies/versioning/usability reasons described in
[1,2,3], this was a bad idea.
*## Reason the Current Behaviour is Bad!*
By auto-generating bundle names, we allow users to have a completely
different mental model from what is actually happening in Brooklyn.
For simple cases (e.g. their .bom file only ever contains one item),
that's fine.
However, it leads to surprising behaviour (and more complicated
Brooklyn code), particularly when using snapshots or forced upgrade.
Consider a (developer) user writing a .bom file, with version
1.0.0-SNAPSHOT containing entities A and B. If the user modifies it
and runs `br catalog add myfile.bom` then we successfully replace the
old auto-generated bundle with this new one. However, if the user
then deletes B (so the .bom contains only A) and does `catalog add`
again, it's unclear to Brooklyn whether this is meant to be the same
.bom file. You could argue that it should be forbidden (because if we
kept the old .bom we'd end up with two different versions of A, and
deleting B would be wrong if it was a different .bom).
The right thing in my opinion is to force the user to have the same
mental model as Brooklyn does: they need to include a `bundle: ` line
in their .bom file so they are explicit about what they are doing.
Note this does not force the user to understand OSGi/java; it just
requires them to think in terms of a "versioned bundle" of catalog
items.
Aled
[1] "Uploading ZIPs for a better dev workflow" dev@brooklyn email thread
[2] "Bundles in Brooklyn" dev@brooklyn email thread
[3] "Bundles of fun" dev@brooklyn email thread
