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

CDN URL support missing #3

Open
destructobeam opened this issue Jun 9, 2018 · 14 comments
Open

CDN URL support missing #3

destructobeam opened this issue Jun 9, 2018 · 14 comments

Comments

@destructobeam
Copy link
Collaborator

destructobeam commented Jun 9, 2018

Hi,

I'm trying to use this gem with fog-rackspace 0.1.5 and uploads are working, but then all the files are referencing the API endpoint http://storage101.syd2.clouddrive.com/v1/MossoCloudFS_922621/ as the base URL when using attachment_url method on the model, which isn't a publicly accessible path, instead of something public.

Carrierwave uses an independent config called asset_host to generate public URLs, and fog-rackspace has the options :rackspace_storage_url and :rackspace_cdn_url, but not sure if those are accessible/which one would be ideal for this use case?

It looks from the following code lines I think that the URL generation is using the API endpoint path, not something public?

def url(id, **options)
signed_url = file(id).url(Time.now + @expires, **options)
if @public
uri = URI(signed_url)
uri.query = nil
uri.to_s
else
signed_url
end
end

Sorry for explaining that terribly.

@janko
Copy link
Member

janko commented Jun 15, 2018

fog-rackspace has the options :rackspace_storage_url and :rackspace_cdn_url, but not sure if those are accessible/which one would be ideal for this use case?

Have you tried if either of those settings work? If fog-rackspace doesn't have a way of setting a CDN URL, then we can do something similar what Shrine::Storage::S3#url does, i.e. replace the original host with the CDN's manually.

@destructobeam
Copy link
Collaborator Author

Thanks. I did try both of those settings in fog-rackspace, neither of which changed the final URL generated. I think that could be why Carrierwave has it's setting outside of it's fog config.

Had a quick look at the S3 adapter code you mentioned and the host option seems like a good way to go, if I am reading that correctly.

@janko
Copy link
Member

janko commented Jun 18, 2018

Looking at the fog-rackspace codebase, it seems that :rackspace_storage_url is actually the one that's going to change the URL host. Try that one and let me know whether it works.

If not, I will go ahead and implement a :host option just like in Shrine::Storage::S3.

@janko
Copy link
Member

janko commented Jun 18, 2018

Actually, no, it seems :rackspace_storage_url will change the URL for all calls, not just for URL generation.

@destructobeam
Copy link
Collaborator Author

destructobeam commented Jun 18, 2018

That almost works, but passing just the domain to :rackspace_storage_url causes excon to crap out on app launch with an Invalid URI error and passing it with an https:// prefix causes the URLs that get generated to the uploads to start with https://https//..., the domain/attachment path after that is correct though.

@janko
Copy link
Member

janko commented Jun 22, 2018

I pushed a2f22b8 to master that adds the :host URL option, let me know if it works. Afterwards I can release a new version.

@destructobeam
Copy link
Collaborator Author

Unfortunately that hasn't worked, getting the following in return from the url method with :host => ENV.fetch('RACKSPACE_CDN_URL')

https://475d73c7ad96e305342c-6963e5ab7a27f792ba81e045bb37ff8e.ssl.cf4.rackcdn.com/v1/MossoCloudFS_922621/match_pick_development/leaderboard/24b0e821-acf3-434b-9bf5-83ccb78f6514/logo/5a56b858c0736a323437c077630641e9.png

Where the v1/MossoCloudFS_922621/match_pick_development/ part is the Rackspace API plus bucket name. Without those included in the URL that is the correct path.

Also would it be a pain to be able to set the CDN URL in the shrine storage initialiser, as I think it would probably be the most used case for it?

Thanks!

@janko
Copy link
Member

janko commented Jun 27, 2018

Oh, that means I would have to have provider-specific code in shrine-fog, because setting the CDN URL would have to know about Rackspace URL. I don't think that's the right way to go, the whole point of Fog is that you don't need to have provider-specific logic, so I would like to draw the line here (I don't want Shrine::Storage::Fog to turn into CarrierWave::Storarge::Fog).

You should the probably make a feature request to the fog-rackspace gem. Alternatively you can create a helper method that modifies the Rackspace URLs to go through CDN.

@janko janko closed this as completed Jun 27, 2018
@janko
Copy link
Member

janko commented Jun 27, 2018

Also would it be a pain to be able to set the CDN URL in the shrine storage initialiser, as I think it would probably be the most used case for it?

The Shrine convention is that URL modifiers are passed to the #url method. That way you can for example have rotating CDN hosts, bypassing browser's 8 requests-per-domain limit (that wouldn't be possible if :host was statically passed to the initializer).

@destructobeam
Copy link
Collaborator Author

Hi, I'm revisiting this after upgrading to Shrine 3, and creating some derivations that now also need a public URL.

It looks as though the S3 storage that ships with Shrine properly forwards the Shrine::UploadedFile#url method to public_url if public is specified in the options, whereas this fog adapter seems to skip this.

In the fog docs here: http://fog.io/storage/#sending-it-out it seems as though public_url is a standard fog storage method.

Any chance we could re-open this one? I could possibly work on a pull request for this?

Thanks!

@janko janko reopened this Jul 13, 2020
@janko
Copy link
Member

janko commented Jul 13, 2020

Thanks, feel free to send a PR 👍

@destructobeam
Copy link
Collaborator Author

Just had a bit of time to pick this up again. The tests need a fair bit of work to accomodate the differences in the providers, so I was just looking to get some feedback if you had a moment. What I have been considering are:

1. My Test Refactoring

  • .env contains AWS provider settings, and is the default provider in the test helper
  • .env.rackspace contains Rackspace provider settings
  • test:aws and test:rackspace are distinct Rake tasks, test task runs all providers
  • Addition of a bunch of helper methods for signed url keys, etc.

2. Abandon Ship

I don't want to do all this just to add a bunch of maintenance burden to your library unless the benefit of having it outweighs that, so happy to close this issue and just continue overwriting the methods I need, if you don't think it's worth it. You would obviously have a much better idea of the value of supporting Fog more generally outside of AWS. With more hosting providers supporting S3 APIs and the fog-rackspace gem lagging behind fog core, this might be the better path?

@janko
Copy link
Member

janko commented Jan 7, 2021

I know very little about Fog, to be honest, and haven't used it in production. If you would like to make some fixes, I will happily give you commit access and gem push access. In this case just let me know what is your RubyGems.org email address.

@destructobeam
Copy link
Collaborator Author

Yeah, no worries. Honestly, unless more coverage is going to benefit others it's not doing well on the cost/benefit front.

That said, I'm happy to be added as committer (and same username on rubygems as here), I can then add more test coverage when I have the spare time. And if someone else sees this issue, and replies here, that will be much more of a motivator.

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

No branches or pull requests

2 participants