Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rackspace|storage] implement get_http_url and get_https_url #2103

Closed
krames opened this issue Aug 29, 2013 · 20 comments
Closed

[rackspace|storage] implement get_http_url and get_https_url #2103

krames opened this issue Aug 29, 2013 · 20 comments

Comments

@krames
Copy link
Member

krames commented Aug 29, 2013

Paperclip currently uses get_http_url and get_https_url to retrieve temporary urls. These methods should be implemented for the Rackspace storage provider.

Here is the relevant line of source from paperclip
https://github.com/thoughtbot/paperclip/blob/master/lib/paperclip/storage/fog.rb#L143

@geemus
Copy link
Member

geemus commented Aug 29, 2013

@krames - yeah. We should probably standardize, as further evidenced here: https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/storage/fog.rb#L139

@krames
Copy link
Member Author

krames commented Aug 29, 2013

@geemus I think your right.

@krames
Copy link
Member Author

krames commented Sep 4, 2013

I did some brief analysis on this. These are all the model level methods to retrieve urls

:atmos=>
  "Files"=>[:get_url],
  "File"=>[:public_url]
:aws=>
  "Directory"=>[:public_url],
  "Files"=>[:get_url, :get_http_url, :get_https_url],
  "File"=>[:public_url, :url]},
:google=>
  "Directory"=>[:public_url],
  "Files"=>[:get_url, :get_http_url, :get_https_url],
  "File"=>[:public_url, :url]},
:hp=>
  "Directory"=>[:public_url, :cdn_public_url, :cdn_public_ssl_url],
  "Files"=>[:get_url, :get_cdn_url, :get_cdn_ssl_url],
  "File"=>
   [:public_url, :cdn_public_url, :cdn_public_ssl_url, :temp_signed_url]},
:rackspace=>
  "Directory"=>[:public_url=, :public_url, :ios_url, :streaming_url],
  "Files"=>[:get_url],
  "File"=>[:public_url, :ios_url, :streaming_url]},
:openstack=>
  "Directory"=>[:public_url],
  "Files"=>[:get_url],
  "File"=>[:public_url]}

The only common method among service providers is public_url. I was going to suggest adding parameters to that method, however, some of the services already accept parameters and it's inconsistent. The only other commonality is between google and aws which share the same set of methods which paperclip currently takes advantage of.

My suggestion is to adopt the google/aws methods if they are sufficient or introduce a new method/hijack an old method that takes a hash allow providers to pass along extra options. What are your thoughts?

@krames
Copy link
Member Author

krames commented Sep 4, 2013

Looping in @samsonjs because of our conversation on issue #2055

@rupakg
Copy link
Member

rupakg commented Sep 4, 2013

Good idea to standardize. I will put in these aliases as well in the HP provider.

@krames
Copy link
Member Author

krames commented Sep 4, 2013

@rupakg Awesome!

@geemus
Copy link
Member

geemus commented Sep 4, 2013

@krames sounds reasonable to me also. If you start down that path and run into any issues or have questions don't hesitate to bug me for assistance though. Thanks!

@samsonjs
Copy link
Contributor

samsonjs commented Sep 5, 2013

I haven't had a chance to start on this yet. If I get to it before you
finish I will collaborate with you, if you want.

-s

On Wed, Sep 4, 2013 at 7:46 AM, Wesley Beary notifications@github.comwrote:

@krames https://github.com/krames sounds reasonable to me also. If you
start down that path and run into any issues or have questions don't
hesitate to bug me for assistance though. Thanks!


Reply to this email directly or view it on GitHubhttps://github.com//issues/2103#issuecomment-23794920
.

@krames
Copy link
Member Author

krames commented Sep 5, 2013

@samsonjs Thanks! I ended up switching gears too. Hopefully I will get to pick this up again sometime soon.

@benjohnson77
Copy link

this would great to get in ... I ran into this today trying to get a full listing of all the ssl URL of each container.

@julweber
Copy link
Contributor

julweber commented Oct 7, 2013

@rupakg I can add aliases for the hp provider so we can use the hp and openstack temp url methods in a common way. What do you think about this?

@rupakg
Copy link
Member

rupakg commented Oct 7, 2013

@julweber Yeah that will work. I can review after you commit.

@julweber
Copy link
Contributor

julweber commented Oct 7, 2013

ok. i will add a pull request tomorrow.

@julweber
Copy link
Contributor

julweber commented Oct 8, 2013

Would you prefer to have an alias for File#url pointing to the old method or should i implement the method in File#url and add an alias with the old method name to keep it backwards compatible?

@geemus
Copy link
Member

geemus commented Oct 8, 2013

Keeping it backwards compatible is preferable, I don't feel too strongly about how you go about doing that though.

@julweber
Copy link
Contributor

julweber commented Oct 8, 2013

my preferred way would be to implement the methods according to the aws and openstack provider interface and add aliases for the old method names

@geemus
Copy link
Member

geemus commented Oct 8, 2013

@julweber - cool, definitely sounds like a good plan.

@julweber
Copy link
Contributor

julweber commented Oct 8, 2013

Done :)
#2226

@krames
Copy link
Member Author

krames commented Oct 30, 2013

get_http_url and get_https_url have been added to all storage service providers that support this functionality.

In the future we might want to explore a more intuitive future proof solution to this problem and deprecating all of the cruft.

@krames krames closed this as completed Oct 30, 2013
@geemus
Copy link
Member

geemus commented Oct 30, 2013

Thanks!

On Wed, Oct 30, 2013 at 8:36 AM, Kyle Rames notifications@github.comwrote:

Closed #2103 #2103.


Reply to this email directly or view it on GitHubhttps://github.com//issues/2103
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants