brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server issue #616: Fixed BlobStoreContextFactoryImpl. It can now ha...
Date Mon, 10 Apr 2017 11:54:18 GMT
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.
---

Mime
View raw message