search

AOEpeople / Aoe_LazyCatalogImages

Magento module to lazy render catalog images
Open Software License 3.0
71 stars 14 forks source link

Reduce filename length further, support legacy filenames #17

Closed toddbc closed 3 years ago

toddbc commented 7 years ago

We have some saved JSON-encoded file paths, so it's more convenient if they just continue working, but I can't be sure they've all been generated.

This also reduces the filename length by another 15% or so in the common case. JSON compresses relatively well.

All 3 variants of the filenames (before 722e8a1a, after 722e8a1a, and with ZLIB) work / are supported for decode.

drobinson commented 7 years ago

Looks good but I'm not well versed in this module. Maybe @thebod or @LeeSaferite can weigh in...

toddbc commented 7 years ago

Oh, forgot to note why it matters: filenames longer than 255 characters will generate a "filename too long" error, i.e. when hosted on NFS.

Mostly this is impacted by the source filename length, which is part of the encoded JSON.

LeeSaferite commented 7 years ago

I think I would ask why you are hosting ephemeral cache files on NFS though. :) One of the primary goals behind this module was to remove the need to host the cached images on NFS for cluster sharing. Since the request is lossless, the URL contains all that is needed to create the image. Each node of a cluster can then rebuild the image the first time it sees the URL.

LeeSaferite commented 7 years ago

I also would highly prefer this new feature be behind a feature flag if possible @drobinson

toddbc commented 7 years ago

Fair, but it's definitely easier (and extremely common) to host all of media/ on NFS.

Not that it matters since the limit for most other filesystems is also 255.

drobinson commented 7 years ago

Good point re feature flag - are you interested in doing that to get this merged @toddbc ?

toddbc commented 7 years ago

I can add that - won't have time until later to verify it. Are you looking for one feature flag, or two (re-enabling support for the syntax pre- 722e8a1a that didn't have a feature flag, and for compression)?

toddbc commented 6 years ago

I haven't been spending much time on Magento 1 lately. I can update this, but I want to avoid too much back and forth that I may not have time for.

Would just the one feature flag be okay?

LeeSaferite commented 3 years ago

Talk about a blast from the past. Sorry for this one just hanging out @toddbc