[openstreetmap/openstreetmap-website] Compress plain GPX uploads with gzip (PR #7124)

[Tom Hughes]

tomhughes left a comment (openstreetmap/openstreetmap-website#7124)

> It's not a monkey patch. The PR introduces a new storage adapter (`ActiveStorage::Service::GpxS3Service`) that inherits from an existing one (`ActiveStorage::Service::S3Service`).

I've looked at this in more detail and while it may not technically be a monkey patch in the traditional sense it's functionally equivalent to one in terms of how it works and the maintenance risk it creates.

As far as I can tell adding your own services is not an intended customisation point in the rails code base, and if it was they would probably have made it possible without having to inject code into their namespace.

That isn't the main problem though - the main problem is that this code is essentially copy and pasted from the code in the existing S3 service, except that for some reason it's been changed from using named parameters to using a generic options argument for most things, and the multipart upload support has been removed, though by default that is only used for files over 100Mb in size so that might be reasonable.

The result though is that any change to the internals of the current S3 service will need to be reflected here which creates a maintenance headache, especially for things that don't cause obvious immediate breakage.

That said I don't see any better way to set the Content-Encoding argument on upload :cry: 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7124#issuecomment-4825759277
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/7124/c4825759277 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260628/72883521/attachment.htm>