brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request: fix location leak - use LocationSpec...
Date Fri, 18 Mar 2016 01:56:48 GMT
GitHub user ahgittin reopened 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 <alex.heneveld@cloudsoftcorp.com>
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 <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-14T19:12:11Z

    Merge branch 'master' into no-localhost-default

commit 40fd76d8124b4f570082f72d3c576b2db05dedf1
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
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 <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-15T12:16:32Z

    misc deprecation updates

commit 2876a9e5884332c38cdd2f631d0d221de2de35c9
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-15T13:27:35Z

    support location spec on entity spec, and deprecate methods which take concrete objects

commit d2c5cc81d1ac784f21c5aa0ebff365c0da6b85b6
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-15T13:28:15Z

    update most tests to pass specs when configuring entity spec

commit 60f470d3b82b8f0d7fec686a0c005627910e100e
Author: Aled Sage <aled.sage@gmail.com>
Date:   2016-03-17T06:48:35Z

    Fix SshMachineLocation cleanup
    
    Previously if a temporary location was created (i.e. using 
    CREATE_UNMANAGED=true) then the cleanup task would execute immediately
    and would close the ssh-connections-pool (potentially while the other 
    thread tried to execute something).

commit f755c25d2e66fa93a4a5f9f29ab8edc8d1c5be48
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-16T20:43:56Z

    first pass rewriting LocationRegistry and resolvers to return specs

commit 3f1e4c61780e67c972ad6ba4785ed5537bbe9108
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
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 <alex.heneveld@cloudsoftcorp.com>
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 b05185eb59dcd6ba2c4f4110ca9a4557f6b8ee0e
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-17T15:09:19Z

    expand REST API to include catalog info, and update REST API for cleaned up LocationSpec
access

commit dfcc6dfc4fd6a6878008e1d31d2409269dce532c
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-17T18:42:10Z

    Merge remote-tracking branch 'upstream/master' into fix-location-leak

commit 41a8d66db06e6b5b97b68d5890dc0a5f68980733
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-17T18:42:16Z

    This closes #67

commit 138e0f66397f57779dc2eb64b14f60081aa2496d
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2016-03-17T18:45:09Z

    Merge branch 'master' into fix-location-leak

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message