GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/70
fix location leak - use LocationSpec wherever possible
This improves how locations are handled internally, switching the resolvers
to return `LocationSpec` rather than `Location` instances which may or may not
be managed.
This allows us to fix a leak if catalog items include a location block (and
includes a test for that, /cc @bostko ). It also simplifies a number of things
internally. There is quite a bit more we'd like to do at some point (/cc
@aledsage):
* have locations come entirely from the `TypeRegistry` (ie the catalog),
doing away with `LocationDefinition` and `LocationRegistry`
* treat `Location` instances *as* `Entity` instances
However those are bigger changes, and this PR addresses the immediate pain
points and makes the code quite a bit tidier.
There are a huge number of tests changed so to facilitate review the main
important commits are:
*
https://github.com/apache/brooklyn-server/commit/2876a9e5884332c38cdd2f631d0d221de2de35c9
pass location *specs* to entity spec (preferred over location instances)
*
https://github.com/apache/brooklyn-server/commit/f755c25d2e66fa93a4a5f9f29ab8edc8d1c5be48
resolvers create location *specs* instead of location instances -- this breaks
Resolver compatibility, mostly pretty easy to fix
*
https://github.com/apache/brooklyn-server/commit/a3d74701fc37ae9b3daae15865510dfb1b94e327
special flag where a spec needs to do something special to create the instance
(beta, used only for PortForwardManager)
*
https://github.com/apache/brooklyn-server/commit/a75e8f7d1b02a8e094f631a82e1cc6ddc4340fec
add catalog info to the object returned via the legacy `/v1/locations`
endpoint to facilitate switching to `/v1/catalog/locations` in future, and notes
Builds on #62.
Note also that this breaks backwards compatibility if someone *implements*
`LocationResolver`. A note should be added in the release notes when this is
merged.
/cc @grkvlt does clocker provide a `LocationResolver`? if so it will need
changed to return a `LocationSpec` not a `Location` instance. if it is not a
straightforward change ping me and I can help; this PR adds a "special
constructor" facility which might apply if a `LocationSpec` doesn't work there
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server fix-location-leak
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/70.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 #70
----
commit d332bcc4d60868736716990660f388c648dd8546
Author: Alex Heneveld <[email protected]>
Date: 2016-03-14T16:53:54Z
does not create a localhost named location any longer. the wizard (coming
soon) will do this for us.
commit bcf4d9b8abb5a1bc79fe52b295dde6e16beadb7b
Author: Alex Heneveld <[email protected]>
Date: 2016-03-14T19:12:11Z
Merge branch 'master' into no-localhost-default
commit 40fd76d8124b4f570082f72d3c576b2db05dedf1
Author: Alex Heneveld <[email protected]>
Date: 2016-03-15T12:10:55Z
add more tests for the number of locations being managed
including a failing test for the case where a blueprint added to catalog
includes a location
commit 515af88879f541e28c907237c7c28a817d0113d0
Author: Alex Heneveld <[email protected]>
Date: 2016-03-15T12:16:32Z
misc deprecation updates
commit 2876a9e5884332c38cdd2f631d0d221de2de35c9
Author: Alex Heneveld <[email protected]>
Date: 2016-03-15T13:27:35Z
support location spec on entity spec, and deprecate methods which take
concrete objects
commit d2c5cc81d1ac784f21c5aa0ebff365c0da6b85b6
Author: Alex Heneveld <[email protected]>
Date: 2016-03-15T13:28:15Z
update most tests to pass specs when configuring entity spec
commit f755c25d2e66fa93a4a5f9f29ab8edc8d1c5be48
Author: Alex Heneveld <[email protected]>
Date: 2016-03-16T20:43:56Z
first pass rewriting LocationRegistry and resolvers to return specs
commit 3f1e4c61780e67c972ad6ba4785ed5537bbe9108
Author: Alex Heneveld <[email protected]>
Date: 2016-03-16T20:44:31Z
update tests to use LocationRegistry spec methods
the CatalogYamlLocationTest.testLocationPartOfBlueprintDoesntLeak recently
introduced to highlight the failure is now fixed,
although there are still port-manager-related failures
commit a3d74701fc37ae9b3daae15865510dfb1b94e327
Author: Alex Heneveld <[email protected]>
Date: 2016-03-17T11:18:22Z
allow a special constructor to be given to specs (beta, needed for
PortForwardManager)
this fixes the port-manager-related failures
commit a75e8f7d1b02a8e094f631a82e1cc6ddc4340fec
Author: Alex Heneveld <[email protected]>
Date: 2016-03-17T15:09:19Z
expand REST API to include catalog info, and update REST API for cleaned up
LocationSpec access
----
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
