rkoeze left a comment (openstreetmap/openstreetmap-website#6476)
Sorry for the late reply here. I think the idea behind the monkey patching is
that it allows timeout exceptions to be propagated without wrapping the
exception (ActiveRecord's default behavior) so we [can handle the timeout case
explicitly](https://github.com/openstreetmap/openstreetmap-website/blob/1515df8ce98e2de3e1cfdbd20681ecbe86cd1417/app/controllers/application_controller.rb#L228).
With that said, `Timeout::Error` is a runtime error, so it looks like
[ActiveRecord should let that bubble
up](https://github.com/rails/rails/blob/dcb29df5b25006dd92f4fc10295f0177a33db93c/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb#L1226)
anyway.
Generally using Timeout to wrap critical operations [is not
recommended](https://www.mikeperham.com/2015/05/08/timeout-rubys-most-dangerous-api/?utm_source=chatgpt.com).
With that in mind, how do we feel about the following?
1. We remove this initializer. It's never worked. And fixing it would lean into
a pattern that is not recommended.
2. We explore setting a database timeout around some of these longer running
queries we're concerned about.
I'm sure I'm missing some historical context here so @tomhughes please correct
me on what I'm missing.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6476#issuecomment-3575449588
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6476/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
