Geo: Avoid having to add every replicator class to a list
Gitlab::Geo::ReplicableModel.replicators
requires all Models which include ReplicableModel to be loaded before being called !35443 (comment 376048633), or else Replicator classes may be missing.
The development environment has config.eager_load = false
by default, unless you enable ENV['RAILS_PROFILE']
.
A Slack convo about this
https://gitlab.slack.com/archives/C7U95P909/p1594316455078000
mkozono: Is this a good idea? I would really appreciate a review so we can progress on !35443 (merged) !36467 (closed)gabriel: I normally don't like to rely on disk traversal... what problem are we trying to fix?
mkozono: The existing
ReplicableModel.replicators
method doesn't work if all the models are not loaded, which I guess can happen in dev/testmkozono: !35443 (comment 376048633)
mkozono: Thanks for the MR comment. I think you're right that this approach is better in an initializer, but I'm not sure it's worth the complexity at that point. I rather vote for hard coding a list of Replicator classes. Thoughts? (Also @valery)
gabriel: we can put the files in autoloader, and that will fix the case where they are not loaded
valery: Yeah, seems like a boring solution
mkozono: Isn't that already happening, but config.eager_load is false in development?
valery: I believe Gabriel meant some (other) preload mechanism
gabriel: no I meant that
gabriel: Last time I checked that we were not default to false in development yet, but I believe even for development there is a work-around/exception list right?
valery: I believe creating exception is easy but I'm not sure we want it because it will require to update it each time we create a new replicator
valery: as we essentially need to load models, not replicators itself
mkozono: in dev, you can set
ENV['RAILS_PROFILE']
to true to makeeager_load
true
gabriel: the list of things we will autoload is basically the "pattern" implemented on the code
gabriel: it's more or less the approach of doing something as an initializer
mkozono: ok, I think since it seems to me that my MR is not truly blocked by this, I'll open a follow up issue to "Resolve sometimes missing Replicator classes in development" and copy this discussion in. WDYT?