[GitHub] brooklyn-server issue #616: Fixed BlobStoreContextFactoryImpl. It can now ha...

[aledsage <gi...@git.apache.org>]

Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/616
  
    LGTM; I also ran the live tests in `org.apache.brooklyn.core.mgmt.persist.jclouds`: `BlobStoreTest` and `BlobStoreExpiryTest`.
    
    It's a shame about the signature `BlobStoreContextFactory.newBlobStoreContext(Location)` - it taking a `ConfigBag` felt nicer, but I see how that doesn't work (without resorting to the `ResolvingConfigBag`, which feels overly complicated). It also means there is less symmetry with `ComputeServiceRegistry.findComputeService(ConfigBag conf, boolean allowReuse)`, but we can live with that as well.
    
    I think we can treat `org.apache.brooklyn.api.location.Location.BlobStoreContextFactory` as being like an internal interface so don't need to worry about backwards compatibility.
    
    A shame we don't have any non-live tests for this stuff. Longer term, we could add something akin to the `StubbedComputeServiceRegistry` but it's certainly not the place of this PR to add that.
    
    Merging now.


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