afeld / mustachio

automatic mustachifying of any image
https://mustachify.me
MIT License
344 stars 93 forks source link

switch to rekognition API v2 #15

Closed afeld closed 11 years ago

afeld commented 11 years ago

http://v2.rekognition.com

simeonwillbanks commented 11 years ago

Hi Aidan,

I'd like to help with the switch.

I forked and read the code. The tests seem out-of-date. After upgrading the VCR cassettes, I got three passing and pended the rest.

❯ bundle exec rake
/Users/simeon/.rbenv/versions/1.9.3-p327-perf/bin/ruby -S rspec spec/unit/analyser_spec.rb spec/unit/job_spec.rb spec/unit/magickly_spec.rb
WARNING: VCR::RSpec::Macros is deprecated. Use RSpec metadata options instead `:vcr => vcr_options`
WARNING: VCR::RSpec::Macros is deprecated. Use RSpec metadata options instead `:vcr => vcr_options`
WARNING: VCR::RSpec::Macros is deprecated. Use RSpec metadata options instead `:vcr => vcr_options`
.****.*.

Pending:
  analysers #face_data_as_px should not scale a photo smaller than 900px
    # #face_data_as_px does not return Hash with 'height' key
    # ./spec/unit/analyser_spec.rb:25
  analysers #face_data_as_px should scale the detection areas proportionally for large photos
    # #face_data_as_px does not return Hash with 'width' key
    # ./spec/unit/analyser_spec.rb:34
  analysers #face_span should return appropriate boundaries
    # TODO : Fix to work with pluggable face detection
    # ./spec/unit/analyser_spec.rb:56
  jobs #crop_to_faces should return a cropped image of the desired size
    # TODO : Fix to work with pluggable face detection
    # ./spec/unit/job_spec.rb:5
  Magickly.face_data should accept a Dragonfly::Job
    # #get_photo returns Dragonfly::Job instance with temp_object=nil
    # ./spec/unit/magickly_spec.rb:22

Finished in 0.19937 seconds
8 examples, 0 failures, 5 pending

Next, I can refactor Mustachio::Rekognition to use http://v2.rekognition.com. After, I could remove pended tests and maybe add some new. Do you have specific plans or thoughts? Any input is appreciated.

Thanks, Simeon

afeld commented 11 years ago

Appears the cassettes are pretty old, yeah. A PR with the updates is very welcome. Thanks!

simeonwillbanks commented 11 years ago

Sounds good. You're welcome. mustache1

simeonwillbanks commented 11 years ago

I read the V2 docs, and the Face Detect URL, parameters and response match Mustachio::Rekognition logic.

I emailed support, and Meng Wang replied:

You should call api via the old one https://rekognition.com/func/api/ v2.rekognition.com is NOT the entry point of our api, and we will NOT maintain this one. Most of our changes in the backend will be transparent to the api clients, so the clients do not need to change any of their existing codes. It very rare that we will change our API protocols. As an exception, we have included the ori_img_size fields recently since it's a popular request from the developers.

Cool!

I'll change the POST URL to use HTTPS; I don't think other changes are required.

I'll give the tests some love, so the PR will also address #13.

Hope this helps :smile:

afeld commented 11 years ago

Wow, thanks for all the legwork! Looking forward to it.

simeonwillbanks commented 11 years ago

HTTPS is working, but I think we should use HTTP. The gem should remain simple; setting up HTTPS was involved. We needed an unreleased version of RestClient (has ssl_version option), OpenSSL 1.0.x, and a specific ssl version.

❯ openssl version
OpenSSL 0.9.8x 10 May 2012
❯ brew install openssl                                       
❯ brew info openssl
openssl: stable 1.0.1e
❯ CONFIGURE_OPTS="--with-openssl-dir=`brew --prefix openssl`"
❯ rbenv install 1.9.3-p194                                   
❯ rbenv local 1.9.3-p194                                   
❯ gem install bundler                           
❯ bundle install                                
# Gemfile
gem 'rest-client', 
  :git => 'git@github.com:Imomoi/rest-client.git',
  :ref => '4d6610b267eb999b4f7aed7ea065b50108414d05'
# lib/mustachio/rekognition.rb
url = 'https://rekognition.com/func/api/'
payload = {
  api_key: REKOGNITION_KEY,
  api_secret: REKOGNITION_SECRET,
  jobs: jobs,
  uploaded_file: file,
  name_space: '',
  user_id: ''
}
response = RestClient::Request.execute(
  verify_ssl: OpenSSL::SSL::VERIFY_PEER,
  ssl_version: :SSLv3,
  method: :post,
  url: url,
  payload: payload
)

The the V2 docs reference http://rekognition.com/func/api/, so we should be good. This is mustaches not money! Thoughts?

afeld commented 11 years ago

Whaa, it was really that much work? The only real advantage to using HTTPS for those API calls is to conceal the key, but I frankly don't care that much. I feel like all of my major Ruby installation issues have been around OpenSSL :poop: OpenSSL 1.0 might not be an option on Heroku Bamboo anyway:

$ openssl version
OpenSSL 0.9.8o 01 Jun 2010
$ ruby -v
ruby 1.9.2p180 (2011-02-18 revision 30909) [x86_64-linux]

I'm also not married to RestClient, if that makes any difference. Whatever works.

simeonwillbanks commented 11 years ago

I've replaced RestClient with Faraday using the Excon adapter. Plus, I've reconfigured VCR to stub the Faraday requests, and the tests are passing.

:metal: We've got HTTPS :metal:

afeld commented 11 years ago

:performing_arts: me the :moneybag: :exclamation: