-
Notifications
You must be signed in to change notification settings - Fork 333
Description
When this gem is used with activerecord it connect to the DB during boot if eager loading is enabled (so eg. in production). This is generally considered bad practice, see for example https://docs.gitlab.com/development/rails_initializers/ . It also causes problems like database rake tasks failing. This is worked around in your code by swallowing a host of errors:
public_activity/lib/public_activity/orm/active_record/activity.rb
Lines 37 to 53 in d820088
| begin | |
| if table_exists? | |
| unless %i[json jsonb hstore].include?(columns_hash['parameters'].type) | |
| if ::ActiveRecord.version.release < Gem::Version.new('7.1') | |
| serialize :parameters, Hash | |
| else | |
| serialize :parameters, coder: YAML, type: Hash | |
| end | |
| end | |
| else | |
| warn("[WARN] table #{name} doesn't exist. Skipping PublicActivity::Activity#parameters's serialization") | |
| end | |
| rescue ::ActiveRecord::NoDatabaseError | |
| warn("[WARN] database doesn't exist. Skipping PublicActivity::Activity#parameters's serialization") | |
| rescue ::ActiveRecord::ConnectionNotEstablished, ::PG::ConnectionBad, Mysql2::Error::ConnectionError | |
| warn("[WARN] couldn't connect to database. Skipping PublicActivity::Activity#parameters's serialization") | |
| end |
However, there's no really great way to replicate automatic setting, and removing it would be a breaking change. One possibility I could see is adding a config which allows setting the column type, and opting out of this logic in that case. Something like PublicActivity.parameters_type = :json. By default this could be nil, and still perform the current check, but if PublicActivity.parameters_type is set, it will use that to determine if serialize needs to be called.
I could prepare a PR if this has any chance of being merged.