HC k8s patches #130
HC k8s patches #130
Conversation
| if options.key?(:shard) | ||
| unless config = configurations[shard_env] | ||
| raise "Did not find #{shard_env} in configurations, did you forget to add it to your database.yml ? (configurations: #{configurations.inspect})" |
There was a problem hiding this comment.
maybe say database config instead of database.yml
There was a problem hiding this comment.
Updated the wording in 2 places.
|
We need to at least make the tests pass. |
a77e562 to
fc21350
Compare
| unless config = configurations[shard_env] | ||
| raise "Did not find #{shard_env} in configurations, did you forget to add it to your database config ? (configurations: #{configurations.inspect})" | ||
| end | ||
| unless shard = config['shard_names'].include?(options[:shard]) |
There was a problem hiding this comment.
this shard variable is not being used, is it?
|
I can only get the migrations to run if I change The problem is that |
| else | ||
| @shard || self.class.default_shard | ||
| end | ||
| if klass.nil? || klass.is_sharded? |
There was a problem hiding this comment.
If we don’t like klass being nil, perhaps we should change the method signature?
There was a problem hiding this comment.
Ah… as long as @shard is set, it works fine.
Also, this is Rails 4 code. Let’s just leave it…
It does not make sense to have a default shard so we're removing this
We don't want to have a "selection" where nothing is selected as that may cause weird issues. For instance it may result in not being able to get the schema of sharded tables.
If we're using ARS we expect shards to be configured. We don't want to silently fail in case the configuration is missing.
If we're missing configuration we raise an exception at the point we discover the missing configuration. This is to avoid accidentally calling .shard= with nil.
We move the state manipulating on_slave= call to after we check that we're actually able to perform the shard switch. Otherwise we may accidentally update state and then see an exception leaving us in an inconsistent state.
When selecting a new shard the user should always select a shard. It does not make sense to select NO_SHARD. With this change we only allow integer names for shards.
When we ask for the shard we must ensure that we do have a shard selected.
We are no longer exclusively using yml for config. Update the wording of the error message to reflect this.
fc21350 to
4915dc1
Compare
|
Superceded by #136 |
This set of patches tighten up what the possible states are for the shard selection. This is done according to the fail-fast principle where we would rather know immediately if something is in a weird state.
Warning: This is a breaking change so we may want a new major release for this