MirrorService needs work

[ghost]

There are two problems with ActiveStorage::Service::MirrorService in the current code:

1. The generated sample config is wrong. Looking at the service code, it should be something like this instead:

mirror:
  service: Mirror
  services: [ local, amazon, google ]

2. Even with that change, the service still doesn't work, because the services attribute gets initialized to an array of strings, when it really needs to be an array of other Services. Quick and dirty fix is to edit the initializer:

  # def initialize(services:)
  #   @services = services
  # end
  def initialize(services:)
    service_array = []
    config_file   = Pathname.new(Rails.root.join("config/storage_services.yml"))
    configs = YAML.load(ERB.new(config_file.read).result)
    services.each do |service|
      if service_configuration = configs[service.to_s].symbolize_keys
        service_name = service_configuration.delete(:service)
        service_array << ActiveStorage::Service.configure(service_name, service_configuration)
      end
    end

    @services = service_array
  end

I'm not putting in a PR at the moment because it's entirely possible the current code authors have a better fix in mind and just haven't gotten around to implementing it yet. Mostly just noting this for anyone else playing with early versions.

[dhh]

Yeah Mirror is half-baked. Need to actually do this YML config munging in the Railstie where the rest of that happens. The classes themselves should just take the as-configured-by-Railtie instances 👍. If you feel like taking a swing at that, please have a crack!

…

On Jul 7, 2017, at 01:19, Mike Gunderloy ***@***.***> wrote: There are two problems with ActiveStorage::Service::MirrorService in the current code: The generated sample config is wrong. Looking at the service code, it should be something like this instead: mirror: service: Mirror services: [ local, amazon, google ] Even with that change, the service still doesn't work, because the services attribute gets initialized to an array of strings, when it really needs to be an array of other Services. Quick and dirty fix is to edit the initializer: # def initialize(services:) # @services = services # end def initialize(services:) service_array = [] config_file = Pathname.new(Rails.root.join("config/storage_services.yml")) configs = YAML.load(ERB.new(config_file.read).result) services.each do |service| if service_configuration = configs[service.to_s].symbolize_keys service_name = service_configuration.delete(:service) service_array << ActiveStorage::Service.configure(service_name, service_configuration) end end @services = service_array end I'm not putting in a PR at the moment because it's entirely possible the current code authors have a better fix in mind and just haven't gotten around to implementing it yet. Mostly just noting this for anyone else playing with early versions. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[ghost]

I opted for a generic concept of services-with-children rather than special case the Mirror service. There's probably some sexy recursive way to write the code but I have trouble conceiving of a good reason to go more than one level deep so I left it unrolled.

[dhh]

Work happening in #16.