migrating from Knox to Amazon's SDK #85
Conversation
| @@ -373,13 +416,16 @@ exports.init = function (grunt) { | |||
| * option declared in the global s3 config. | |||
| */ | |||
| exports.sync = function (src, dest, opts) { | |||
| console.log('testing synC'); | |||
There was a problem hiding this comment.
I was trying to figure out why the sync code wasn't running. Leftover stub from that. It shall be removed shortly.
… since it was Knox specific
|
Uploads fail with |
|
@c2h5oh I don't think those formats are right. It expects a UTC string. Example: 'Expires' : new Date(Date.now() + 315360000000).toUTCString() |
|
Based on the error message I've quoted above all three formats should be acceptable. |
|
Hmm, I don't see any of the supported formats in the knox or grunt-s3 documentation. I'm wondering if the knox Expires header is more accepting of various input formats. The aws-sdk that backs my branch is more strict. @pifantastic should we bump the major version number to indicate API breaking changes, or would you prefer to try to shim all of the differences between knox and aws-sdk? I'm happy to help with this but I haven't heard any feedback from you, and I've put a lot of time into this patch already. I'd like to get your thoughts on this before I dump a bunch more time into it. |
|
+1 for version bump Drew Wilson On Tue, Aug 13, 2013 at 11:05 AM, Mike Reinstein
|
|
How difficult would it be to make expires compatible with knox? If that's the only breaking change, I'd rather not do a minor version bump. |
|
I'd also like to point out that this is great work @mreinstein and I really appreciate it. |
|
I think this PR is needed for #72 via (http://docs.aws.amazon.com/AWSJavaScriptSDK/latest/frames.html) needing to know what's actually in the bucket to remove it. Something Knox does not have the ability to do AFAIK [edit] http://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketGET.html |
|
Sorry for the delay in reply, crazy week!
Probably not hard.
The thing that scares me the most is that grunt-s3 is to a large extent a pass-through to the underlying file upload library. I suspect a lot of the allowed inputs that knox uses are different from aws-sdk. We could shim these, and I'm worried about the possibility of certain calls supporting features that mysteriously go away or change when we move to aws-sdk. It might be safer to declare API breakage rather than put a lot of work into shimming, and still have breaking changes leak through inadvertently. That's just a gut feeling though, open for discussion. Thoughts? |
My pleasure! Great library, happy to help. |
|
Hey, this looks good. Any news? I'd love to get rid of |
|
@pifantastic any thoughts on this? |
|
Hey guys @pifantastic is trying to take a break here, I'm sorry I'm behind on reviewing pull requests. This one seems to be urgent, so I'll make this high priority. |
|
@toxigenicpoem where we left off was a discussion on whether to try to shim up the current behavior and add this as a patch, or to declare it as a breaking change, and increment the version number. My feeling is that given how the behavior of the underlying file transport library is a bit different, it might be wise to declare a breaking change. However if we decided to do this, we should lump in any other breaking API changes into one release, so that we're responsibly following semver and not bumping the major version too often. I was hoping to hear from @pifantastic on this, but maybe you can sub in with your thoughts. |
|
IMHO this should be a major version change, but this is definitely not ready for release at this point:
Maybe other stuff I haven't used is broken too - the core functionality: upload/download is solid - I've been using it in production for 2+ months I am not aware of any other breaking changes that are waiting to be merged. |
|
+1 any updates on this? |
#83