Deprecate 'master' in favor of 'primary' #290
Conversation
mlarraz
force-pushed
the
primary
branch
3 times, most recently
from
11212cd to
d3c5775
Compare
| @makara_config[:connections].map do |connection| | ||
| base_config.merge(makara_config.except(:connections)) | ||
| .merge(connection.symbolize_keys) | ||
| @all_configs ||= @makara_config[:connections].map do |connection| |
There was a problem hiding this comment.
This seems like a behavior change. Now we won't be re-calculating all_configs each time it is called. That may ultimately be desired behavior but wanted to call out that potentially this could break things
There was a problem hiding this comment.
AFAIK these are immutable structures anyway (since they're just loaded from config files)
| # do these args require a master connection | ||
| def needs_master?(method_name, args) | ||
| true | ||
| # Do these args require a primary connection | ||
| def needs_primary?(method_name, args) | ||
| if respond_to?(:needs_master?) | ||
| warn "#{self.class}#needs_master? is deprecated. Switch to #needs_primary?" | ||
| needs_master?(method_name, args) | ||
| else | ||
| true | ||
| end |
There was a problem hiding this comment.
I don't fully understand this change. How could it respond_to needs_master? since your removed that method? It also seems like it would be throwing a deprecation warning potentially when using the correct method. I'm sure there is some method magic happening that I just don't see but wanted to call it out.
There was a problem hiding this comment.
My understanding is this method is meant to be overwritten in subclasses by users. So if someone has defined their own needs_master?, we should still call it but emit this warning that they should rename the method.
Also deprecates the use of the
slaverole (although this doesn't actually matter since we just assume anything butprimaryis a replica).