afeld / magickly

image manipulation as a (plugin-able) service
http://magickly.afeld.me
MIT License
179 stars 35 forks source link

Timeouts when using magickly on local images #7

Closed xavierRiley closed 12 years ago

xavierRiley commented 12 years ago

I've mounted magickly in a Padrino app (basically Sinatra) using the following in config.ru


#!/usr/bin/env rackup
# encoding: utf-8

# This file can be used to start Padrino,
# just execute it from the command line.

require File.expand_path("../config/boot.rb", __FILE__)

require 'rack/cache'

use Rack::Cache

map '/magickly' do
  run Magickly::App
end

map '/' do
  run Padrino.application
end

I can run this locally using bundle exec thin start and it works fine.

The problem is when I try and use magickly on an image that's hosted locally, like so;

http://magickly.dev:3000/magickly/?src=/images/logo.png
http://magickly.dev:3000/magickly/?src=http://0.0.0.0:3000/images/logo.png

Neither of these work and they fail with Timeout::Error

Any ideas?

afeld commented 12 years ago

I've came across this too and was surprised at the solution: use WEBrick instead of Thin (at least locally). Magickly is making an HTTP request back to the same server, but Thin will only serve one request at a time - WEBrick is able to handle that recursive request. Assuming you have multiple app instances available in production (Passenger, a Thin cluster, etc.), it won't be an issue.

xavierRiley commented 12 years ago

For anyone else who comes across this, I've been trying it out with Padrino and have the following workaround.

If you put the following in the lib folder

module Magickly
    class << self

    def process_src(src, options={})
      raise ArgumentError.new("src needed") if src.blank?

      escaped_src = URI.escape(src)

      if File.exists?(Padrino.root('public', Pathname.new(URI.parse(escaped_src).path).cleanpath.to_s))
        image = Magickly.dragonfly.fetch_file(File.open(Padrino.root('public', Pathname.new(URI.parse(escaped_src).path).cleanpath.to_s)))
      else
        image = Magickly.dragonfly.fetch(escaped_src)
      end

      process_image(image, options)
    end
end

This checks if the file exists within the Padrino public folder (with Pathname cleaning any relative url funny business) and if so, serves it normally from the local filesystem. I don't know whether this could be applied to the core of Magickly, but it should be easy to do something similar with a Rails or Sinatra app.

Do you think this could be abstracted out a bit and included into the core? Perhaps a ENV['PUBLIC_FOLDER_PATH'] variable or similar?

Hope this helps someone.

afeld commented 12 years ago

That is more efficient since it's opening the file directly (and gets around the recursive request problem I mentioned above), but did using WEBrick not work?

xavierRiley commented 12 years ago

WEBrick is fine, it's just my aim here was to have a single app I could deploy to Heroku with Magickly "built in". Also, my Padrino app uses Sinatra synchrony + Event Machine so thin is a requirement for me. I looked into using a thin cluster but I thought the solution above would be easier. Thanks for confirming the issue and helping me out though.

afeld commented 12 years ago

Ah, very good point. You're right - a config option for local files is a good plan. I'll try to put it in soon.

xavierRiley commented 12 years ago

Cool, look forward to seeing it! Thanks.