This repository has been archived by the owner on Nov 24, 2021. It is now read-only.
Replace Knox with aws-sdk #35
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It’s easy enough to make this work the way knox did and the check was looking in the wrong place for the `path` option so this never worked.
… rather than a gobal client
# Conflicts: # index.js
This was referenced Jun 15, 2018
Closed
|
+1 for this. Nice work. |
dominikwilkowski
suggested changes
Jun 16, 2018
index.js
Outdated
| var s3options = assign({}, this.options, { bucket: file.bucket }); | ||
| return knox.createClient(s3options); | ||
| S3Adapter.prototype._resolveBucket = function (file) { | ||
| if (file && file.bucket && file.bucket) { |
There was a problem hiding this comment.
probably don't need file.bucket twice here?
| @@ -10,8 +10,8 @@ | |||
| }, | |||
There was a problem hiding this comment.
We should version bump the package no? "version": "2.0.0", on line 3.
dominikwilkowski
approved these changes
Jun 18, 2018
This was referenced Jun 18, 2018
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
The Knox library which this package was previously based on has gone unmaintained for some time and is now failing in many scenarios. This version replaces Knox with the official AWS Javascript SDK.
Breaking changes
The option
headershas been replaced withuploadParams. If you were setting custom headers with previous version of the S3 Storage Adapter you will need to change these to use the appropriateparamas defined in the AWS DocumentationFor example,
{ headers: { 'x-amz-acl': 'public-read' } }should now be{ uploadParams: { ACL: 'public-read' } }.Additions
{ publicUrl: "https://xxxxxx.cloudfront.net" }) or by passing a function which takes thefileobject and returns a the url as a string.Other
pathto have a leading slash has been removed. The previous implementation failed to catch this miss-configuration and Knox helpfully made the file uploads work anyway. This has lead to a situation where it is possible/likely that there are existing installations where a miss-configured path is stored in the database. To avoid breaking these installs we now handle adding or removing the leading slash as required.Thanks
Thanks to @aaronfranco, @stevenkaspar, @alsoicode and @sktt who all had a go at this refactor before me. I have borrowed heavily from all of your work.