-
Notifications
You must be signed in to change notification settings - Fork 403
Added s3_uploads_s3_client filter to be able to change the S3Client #603
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
base: master
Are you sure you want to change the base?
Added s3_uploads_s3_client filter to be able to change the S3Client #603
Conversation
|
@SeBsZ are you not able to do this via |
|
Closing as no response. |
|
Sorry, I didn't see your message before @joehoyle . We can't do this with |
|
@SeBsZ those should also be part of |
|
@joehoyle I finally had time to dive back into my code and figured out why we couldn't use What we needed was the ability to override and switch client params at potentially every S3 call. We have a script that goes through all the files we have stored on S3 servers (which may be different servers across the world, in different buckets). When the script loops through the files, it needs to be able to switch client params on the fly. We did this by adding our own filter in Hopefully this makes more sense now. Let me know if you can think of another way we could do this. |
|
Ahh ok, yes that makes sense, yes I think we do this for performance -- however I can see the argument for having a filter you can change on an ongoing basis. I think you should also achieve this by provider your own S3Client subclass which does whatever logic you would want on a dynamic basis. |
|
@joehoyle So do you think there is a chance this PR will get merged upstream in its current form? |
We added the ability to filter the S3Client returned by the s3() function. This allows us to switch the S3Client and provide it with different credentials, region and endpoint. We have a site where not all files are stored on the same S3 provider, and using the sample function below then gives us the ability to load another set of credentials. It would be great if you could merge this functionality in the main repo.
Sample function: