Add Rails.application.deprecators

[jonathanhefner]

This adds a Rails.application.deprecators method that returns a managed collection of deprecators.

Individual deprecators can be added and retrieved from the collection:

Rails.application.deprecators[:my_gem] = ActiveSupport::Deprecation.new("2.0", "MyGem")
Rails.application.deprecators[:other_gem] = ActiveSupport::Deprecation.new("3.0", "OtherGem")

And the collection's configuration methods affect all deprecators in the collection:

Rails.application.deprecators.debug = true

Rails.application.deprecators[:my_gem].debug
# => true
Rails.application.deprecators[:other_gem].debug
# => true

Additionally, all deprecators in the collection can be silenced for the duration of a given block:

Rails.application.deprecators.silence do
  Rails.application.deprecators[:my_gem].warn    # => silenced (no warning)
  Rails.application.deprecators[:other_gem].warn # => silenced (no warning)
end

[rafaelfranca]

Maybe, but I'd not do it now and only do it later if it makes sense for the framework's own usage.

[rafaelfranca]

I think it makes sense.

[p8]

Hi @jonathanhefner.
I'm working on this weeks newsletter and want to include this PR.
Could you explain the use-case for this?
Is it for deprecating gem versions used by Rails?
So for example if we stop supporting dalli version 2, we add:
Rails.application.deprecators[:dalli] = ActiveSupport::Deprecation.new("2.0", "dalli")

[jonathanhefner]

@p8 Close, but it would be Dalli registering Rails.application.deprecators[:dalli] and generating warnings using that deprecator instance.

Before this PR, if a library created its own deprecator instance, users of the library would have to seek out and configure that instance separately from the global ActiveSupport::Deprecation instance. For example, if a library had:

MyGem.deprecator = ActiveSupport::Deprecation.new("2.0", "my_gem")

and a user wanted to set behavior = :raise for all deprecators, they would have to write:

ActiveSupport::Deprecation.behavior = :raise
MyGem.deprecator.behavior = :raise

And so on for each library with its own deprecator instance.

With this PR, each library can have an initializer like:

module MyGem
  class Railtie < ::Rails::Railtie
    initializer :deprecator do |app|
      app.deprecators[:my_gem] = MyGem.deprecator
    end
  end
end

And then the user will only have to write:

Rails.application.deprecators.behavior = :raise

Alternatively, if the user wants to change the behavior for a single deprecator, they now have a standard location to do so:

Rails.application.deprecators[:my_gem].behavior = :raise

And all the above also applies to the debug, disallowed_behavior, and silenced settings, as well as silence blocks.

[p8]

Ok clear. Thanks @jonathanhefner !

[fxn]

I am thinking of a use case that may not fit well with the current design/APIs.

I am implementing support for deprecating associations:

class User < ApplicationRecord
  has_many :posts, deprecated: true
end

The idea is that whenever the association is used (say, User.includes(:posts).where(...)), you get a warning. That would be a preliminary step before being able to delete an association.

OK. The deprecation is checked and issued by AR, but it is not a deprecation of the library itself, nor does it have an horizon. It is an application-level deprecation issued by AR.

Does it make sense to extend deprecations to support this use case?

[fxn]

I have this working as a monkey patch in a client. That implementation just publishes an AS notification, so it is the subscriber in the application who has full control about what to do. It has also the benefit of receiving the reflection in the payload, instead of a string.

I like the flexibility of this approach, it has been helpful in different ways. But, while upstreaming, I am wondering if users would expect a regular deprecation instead.

[jonathanhefner]

@fxn

OK. The deprecation is checked and issued by AR, but it is not a deprecation of the library itself, nor does it have an horizon. It is an application-level deprecation issued by AR.

Does it make sense to extend deprecations to support this use case?

I think it makes sense. Thinking of deprecator objects as communication channels, it seems reasonable to have a deprecator for each "class" of message a user may be interested in. So we could have a Rails.application.deprecators[:active_record_associations] deprecator that a user could configure independently of Rails.application.deprecators[:active_record].

I like the flexibility of this approach, it has been helpful in different ways. But, while upstreaming, I am wondering if users would expect a regular deprecation instead.

I guess it depends on how the user wants to track / handle the deprecation. In particular, do they want the DX of a regular deprecation when running their test suite?

[fxn]

I guess it depends on how the user wants to track / handle the deprecation. In particular, do they want the DX of a regular deprecation when running their test suite?

In my real usage, in production there is no warning issued. Metadata about the reflection and santizied backtrace are sent to Datadog as a structured event.

Use case is that I want to delete the association, CI issues no warnings, but I want to watch production before deleting.

Logs are capped due to their volume, so a warning could be dropped and you'd miss usage. Additionally, the event allows for fine structured queries in Datadog.

[fxn]

One possibility would be a config option per environment that can be :warn or :notify.

[fxn]

Though, I'd like to explore if I can make an application deprecator receive a payload with a reflection object.

[jonathanhefner]

Though, I'd like to explore if I can make an application deprecator receive a payload with a reflection object.

One way would be as an additional argument to the behavior callback, but we currently accommodate multiple arities, and adding another argument might be challenging. 😬

Another way would be to pass the payload as the first argument (e.g. as { message: "...", reflection: x }), but obviously that has backward compatibility concerns.

Perhaps we could add an attribute to ActiveSupport::Deprecation that indicates whether the deprecator's behavior callbacks support rich payloads. The default would be false, in which case, if the deprecator receives a Hash, it would extract :message before passing it to behavior callbacks. Otherwise, the deprecator would pass the payload as is. (And we would update the DEFAULT_BEHAVIORS callbacks to handle either case.)

[fxn]

Yes, I saw that support for multiple signatures which complicates things.

I have not thought about this in detail, but an idea that I had in mind was to introduce a new type of deprecator, an application deprecator. Because here, the responsibilities are kind of reversed, it is not Active Record deprecating something, it is the application deprecating something, only the logic for triggering that is within AR.

So, an application deprecator would not have a horizon, and would have some API that receives a payload. The deprecator would be responsible for warning, or sending an event, building a usage backlog in CI, or whatever logic it wants.

That is a raw idea, don't know if it is any good.