aws-lambda-image
aws-lambda-image copied to clipboard
ysugimoto
config option to change the extension
In previous versions the filename remained the same across backups/reduces/resizes.
In commit f9eed2f1f5b6f89bb6ff8fc849951f0804c13379 the code was changed to change to the file type's extension, breaking my app (when I updated). I don't expect the filenames to change (even if they are wrong).
Made it optional with a config option changeExtension, that defaults to false (the original behavior). I know it's back and forth on 'breaking changes' but hopefully there are more people who haven't yet updated than there are people who have (or had to revert to the old code after finding it breaks their app).
Will change to Boolean(), waiting for the tests to finish because I think there's another failing test and I can't run it on my machine for some reason.
How will it bother ImageMagick? He gets the raw data (see https://github.com/ysugimoto/aws-lambda-image/blob/9924bc179fbc3567d97f86eef8ae94915dd51034/lib/ImageResizer.js#L34) he doesn't know what filename you'll save it to. All that really matters with image types is the mime type set in S3 metadata, which this project does properly. Nobody looks at the filename, only the content type received from the HTTP web server (which S3 chooses according to the metadata).
Yes, ImageMagick doesn't have to know. ImageData.js detects MIMEType from raw data using image-type package. If we do reduce after resize, Is there the resized image's raw data like JPEG format, but raw data is PNG? I concern about this.
If this is clearly, let's merge it.
It should be fine, the type of the data (chosen by ImageMagick and determined by image-type) is completely unrelated to the output filename chosen, which can be anything and doesn't affect the data.
A browser downloading the image from S3 and showing it in an <img> tag would not care about the filename (URL) either, it only looks at the Content-Type header of the response which is sent by S3 according to the mime data set on the file, S3 doesn't care about the filename either.
The only problem is if someone takes the S3 file and determines the file type according to the extension (which is a bad way to determine), in this case the package's user can use the changeExtension option.
Can you check why travis is stuck? I think there might be one failing test I need to fix before this is merged.
Changing you app to make it expect change of the extension would seem to be more reasonable approach for me :)
@kdybicz that would mean the lambda needs to update the DB (Mongo in my case). I actually have all images uploaded as "image.jpg" (to different directories) and I prefer it not to change ever :smile:
@ysugimoto I've been wrapping my brain around these failures and I have no clue 😢
I can barely run the test suite, I have to disable eslint and then I get a lot of Error: write EPIPE. Would also be nice to be able to run a single test. Any ideas?
@odedniv I saw Travis's log, some assertions are failed. And eslint error reports in your environments? An error of write EPIPE might be a process I/O error in node.js, and I'm not sure why, or when that error occurs 😢
Anyway, you can run a single test to install ava globally, I think:
$ npm install ava -g
$ ava test/e2e-jpeg.js
This is the output I get from running the tests:
oded@plex ~/devel/aws-lambda-image $ ava test/e2e-jpeg.js
Reducing to: in-place
Reducing to: some
Backing up to: in-place
Resizing to: in-place
Resizing to: in-place
Resizing to: in-place
Resizing to: in-place
Resizing to: in-place
Reducing to: in-place
Reducing to: in-place
Reducing to: in-place
Reducing to: in-place
Reducing to: in-place
1 passed
3 exceptions
Uncaught Exception
Error: write EPIPE
Uncaught Exception
Error: write EPIPE
✖ Test results were not received from test/e2e-jpeg.js
It doesn't make sense on many levels... for example the file contains more than 4 tests
Hmmmmm... I just run test as same as you, success it.
So, I'm using node v6.9.1 on macOS. Please let me know about your environment? In addition, please reinstall ImageMagick if you can.