Reduce runtime of optimize-image script #456

Closed
snedadah wants to merge 18 commits from shahanneda/optimze-image-script into main
Owner

Closes #441.

  • Changed optimize-images script to keep track of all the images it has already optimized (through a hash) in a already-optimized-images.json file.

  • Removed the public/images folder from gitignore so that optimized images will be tracked.

  • Added better logging for the script

  • Fixed issue with script ignoring images that end with ".JPG" (uppercase)

If reviewers want, I can try to revert the images commit so the number of files changed is reduced, however, I don't think its that bad, if you scroll down all the way once and click the "show more" button one time its pretty easy to get to the changed code files.

Usage:

Now, the ideal case is everytime someone adds a new image, they should run npm run build:images locally, and commit that.

However, if they forgot, that is fine, however, after a while if many images build up and we notice the build time going up again, someone should run npm run build:images and commit that to cache the optimized images.

staging: https://csclub.uwaterloo.ca/~a3thakra/csc/shahanneda/optimze-image-script/about/team/

in the future, we should consider properly storing images in a cdn

Closes #441. - Changed optimize-images script to keep track of all the images it has already optimized (through a hash) in a `already-optimized-images.json` file. - Removed the public/images folder from gitignore so that optimized images will be tracked. - Added better logging for the script - Fixed issue with script ignoring images that end with ".JPG" (uppercase) If reviewers want, I can try to revert the images commit so the number of files changed is reduced, however, I don't think its that bad, if you scroll down all the way once and click the "show more" button one time its pretty easy to get to the changed code files. Usage: Now, the ideal case is everytime someone adds a new image, they should run `npm run build:images` locally, and commit that. However, if they forgot, that is fine, however, after a while if many images build up and we notice the build time going up again, someone should run `npm run build:images` and commit that to cache the optimized images. staging: https://csclub.uwaterloo.ca/~a3thakra/csc/shahanneda/optimze-image-script/about/team/ in the future, we should consider properly storing images in a cdn
snedadah added 4 commits 10 months ago
6f6510a5ee
Reduced number of workers
5263c6880d
added console logging to image script
4f79c73cd3
Ran optimize image script
snedadah added 2 commits 10 months ago
snedadah added 3 commits 10 months ago
snedadah added 2 commits 10 months ago
snedadah added 2 commits 10 months ago
snedadah added 1 commit 10 months ago
bead08ed09
updated warning
snedadah added 1 commit 10 months ago
a3bd8da8cc
Updated env check
snedadah added 1 commit 10 months ago
be527c382e
Updated ci check
snedadah added 1 commit 10 months ago
b6129b3843
cleaned up
snedadah added 1 commit 10 months ago
366e226520
Updated location of script
snedadah changed title from WIP: Reduce runtime of optimize-image script to Reduce runtime of optimize-image script 10 months ago
snedadah requested review from n3parikh 10 months ago
snedadah requested review from a3thakra 10 months ago
snedadah requested review from j285he 10 months ago
snedadah requested review from a258wang 10 months ago
a3thakra reviewed 10 months ago
import { ImagePool } from "@squoosh/lib";
import fse from "fs-extra";
import { default as getImageDimensions } from "image-size";
import SparkMD5 from "spark-md5";
Collaborator

Probably not very dangerous in this scenario, but we should just avoid md5 altogether since it's a broken hashing algorithm. Use a safer algorithm like sha256. You can use the nodejs builtin crypto library to do so. :)

Probably not very dangerous in this scenario, but we should just avoid md5 altogether since it's a broken hashing algorithm. Use a safer algorithm like sha256. You can use the nodejs builtin crypto library to do so. :)
Poster
Owner

Oh true, however my reasoning behind using MD5 is that it is one of the fastest hashing algorithms, and I doubt the unsafeness really effects our usecase.

Oh true, however my reasoning behind using MD5 is that it is one of the fastest hashing algorithms, and I doubt the unsafeness really effects our usecase.
Collaborator

The problem with checking in the public/images folder is that nothing is stopping anyone to add on to that folder.

It would be much better if we could:

  1. restore public/images artifact from a previous CI run before optimizing images
  2. run this optimize images script
  3. create a new public/images artifact and to overwrite the previous one

This was we are never storing 2x the images in our repo and it avoids all manual work while decreasing build times.

cc @r389li can we setup artifactory or something and add it to drone? https://plugins.drone.io/plugins/artifactory

The problem with checking in the public/images folder is that nothing is stopping anyone to add on to that folder. It would be much better if we could: 1. restore `public/images` artifact from a previous CI run before optimizing images 1. run this optimize images script 1. create a new `public/images` artifact and to overwrite the previous one This was we are never storing 2x the images in our repo and it avoids all manual work while decreasing build times. cc @r389li can we setup artifactory or something and add it to drone? https://plugins.drone.io/plugins/artifactory
Owner

@a3thakra we definitely don't need Artifactory. What we can do is put the images in /srv on Caffeine, and then just link to them using Apache URL rewrites. Let me know if you have other ideas.

@a3thakra we definitely don't need Artifactory. What we can do is put the images in `/srv` on Caffeine, and then just link to them using Apache URL rewrites. Let me know if you have other ideas.
snedadah closed this pull request 10 months ago

Reviewers

n3parikh was requested for review 10 months ago
a3thakra was requested for review 10 months ago
j285he was requested for review 10 months ago
a258wang was requested for review 10 months ago
All checks were successful
continuous-integration/drone/push Build is passing
Required
Details
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#456
Loading…
There is no content yet.