Skip to content

Commit

Permalink
Merge #330
Browse files Browse the repository at this point in the history
330: Fix settings update regression r=ellnix a=ellnix

# Pull Request

## Related issue
Fixes #329
May fix #328

## What does this PR do?
- `SafeIndex` before I tried to fix #280 with #301 did not guarantee that its index exists at all, since ``@index`` was created asynchronously it was possible that `SafeIndex#settings` could be called on an index that does not exist.
  This line:
  https://github.com/meilisearch/meilisearch-rails/blob/29f59c88881b5a4b5a03d990f11d3aac220cd367/lib/meilisearch-rails.rb#L309 
  was supposed return an empty hash when asked to fetch the settings of an index that does not exist, however `ApiError#code` is the meilisearch code (`"index_not_found"`) and not the http code (`404`). That line was therefore skipped and the `index_not_found` error was being propagated and caught by the `rescue nil` in:
  https://github.com/meilisearch/meilisearch-rails/blob/e5ad4d1f10c078097bf211e4d3a6e1d48b24bbc5/lib/meilisearch-rails.rb#L750
  until I removed it and tried to replace it with ensuring that `SafeIndex` had an index by making `create_index!` synchronous and all kinds of hell broke loose since now every time an index was used would cause a synchronous wait.



Co-authored-by: ellnix <[email protected]>
  • Loading branch information
meili-bors[bot] and ellnix committed Feb 19, 2024
2 parents 29f59c8 + 03a8daa commit b0bfab1
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
6 changes: 3 additions & 3 deletions lib/meilisearch-rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def initialize(index_uid, raise_on_failure, options)
@raise_on_failure = raise_on_failure.nil? || raise_on_failure

SafeIndex.log_or_throw(nil, @raise_on_failure) do
client.create_index!(index_uid, { primary_key: primary_key })
client.create_index(index_uid, { primary_key: primary_key })
end

@index = client.index(index_uid)
Expand Down Expand Up @@ -306,7 +306,7 @@ def settings(*args)
SafeIndex.log_or_throw(:settings, @raise_on_failure) do
@index.settings(*args)
rescue ::MeiliSearch::ApiError => e
return {} if e.code == 404 # not fatal
return {} if e.code == 'index_not_found' # not fatal

raise e
end
Expand Down Expand Up @@ -843,7 +843,7 @@ def meilisearch_settings_changed?(server_state, user_configuration)
if user.is_a?(Hash) && server.is_a?(Hash)
meilisearch_settings_changed?(server, user)
elsif user.is_a?(Array) && server.is_a?(Array)
user.map(&:to_s) != server.map(&:to_s)
user.map(&:to_s).sort! != server.map(&:to_s).sort!
else
user.to_s != server.to_s
end
Expand Down
11 changes: 5 additions & 6 deletions spec/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1136,23 +1136,22 @@
let(:index_instance) { instance_double(MeiliSearch::Index, settings: nil, update_settings: nil) }
let(:slow_client) { instance_double(MeiliSearch::Client, index: index_instance) }

before do
allow(slow_client).to receive(:create_index)
allow(MeiliSearch::Rails).to receive(:client).and_return(slow_client)
end

it 'does not raise error timeouts on reindex' do
allow(index_instance).to receive(:add_documents).and_raise(MeiliSearch::TimeoutError)
allow(slow_client).to receive(:create_index!).and_return(index_instance)

allow(MeiliSearch::Rails).to receive(:client).and_return(slow_client)

expect do
Vegetable.create(name: 'potato')
end.not_to raise_error
end

it 'does not raise error timeouts on data addition' do
allow(slow_client).to receive(:create_index!).and_raise(MeiliSearch::TimeoutError)
allow(index_instance).to receive(:add_documents).and_return(nil)

allow(MeiliSearch::Rails).to receive(:client).and_return(slow_client)

expect do
Vegetable.ms_reindex!
end.not_to raise_error
Expand Down

0 comments on commit b0bfab1

Please sign in to comment.