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

> > Why do we need to monkey patch rails?
> 
> 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`).

Do you know if there's any documentation on writing new adapters like this? 
Does it need to be in the rails owned `ActiveStorage` namespace, or could it be 
somewhere else?

> > Can somebody explain how GpxS3Service even gets invoked?
> 
> Set `service: GpxS3` in `storage.yml`. I think that the new adapter exists in 
> the ActiveStorage namespace so that it can be referenced here. No idea if 
> different naming is possible. This PR is missing a change to the example file 
> and a mention in the docs.

So this doesn't actually use this by default, we will need to enable it in our 
production environment? What happens if somebody uses S3 and doesn't enable it? 
The traces still get compressed but don't get automatically decompressed on 
download?

> It's true that the code changes make the objective of this PR a bit unclear. 
> The objective is making it so that we compress GPX files on upload (unless 
> already compressed), in such a way that doesn't require us to decompress them 
> back when serving them, because we delegate decompression to browsers via 
> `Content-Encoding`.
> 
> For comparison, I have a GPX file here (taken from the website at random) 
> that compresses from 341kB to 24kB. Having said that, I don't know what the 
> storage impact would be overall as I don't know how frequently users upload 
> plain GPX files as opposed to compressed ones.

I understand the goal is to compress otherwise uncompressed traces.

What I'm questioning is whether we need to automatically decompress them give 
we already have compressed traces and don't do that for them.


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

Message ID: 
<openstreetmap/openstreetmap-website/pull/7124/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to