Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement wait_for_pending_update method #43

Merged
merged 3 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion lib/meilisearch/index.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# frozen_string_literal: true

require 'meilisearch/http_request'
require 'timeout'

module MeiliSearch
class Index < HTTPRequest
class Index < HTTPRequest # rubocop:disable Metrics/ClassLength
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With rubocop we chose not to use this kind of line in the code: we use the .rubocop_todo.yml file instead. This file is autogenerated.
When you're done with the linter errors (I mean, when all the remaining errors are acceptable for you) you can generated the file with the command bundle exec rubocop --auto-gen-config.
Stripe uses it the same way 😉
https://github.com/stripe/stripe-ruby/blob/52f64b2bacf6caa23dba15cf43b723075c14d133/.rubocop_todo.yml

Copy link
Member

@curquiza curquiza Jun 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an obvious way for new users, I'm going to add some explanation in the README 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

attr_reader :uid

def initialize(index_uid, url, api_key = nil)
Expand Down Expand Up @@ -231,5 +232,16 @@ def accept_new_fields
def update_accept_new_fields(accept_new_fields)
http_post "/indexes/#{@uid}/settings/accept-new-fields", accept_new_fields
end

def wait_for_pending_update(update_id, timeout_in_ms = 5000, interval_in_ms = 50)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this method in the ### UPDATES part, and not at the end of this file?

Timeout.timeout(timeout_in_ms.to_f / 1000) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather throw a custom MeiliSearch Exception so that the user knows it comes from MeiliSearch. You have to add an error in the error.rb file.
We do the same in JS and PHP for example:

(I realized we didn't do that for the python, we have to fix that, but it's normal because when we created this method, the error handler didn't exist ^^)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did implement a custom error, also with a custom message, let me know what do you think :)

loop do
get_update = get_update_status(update_id)
return get_update if get_update['status'] != 'enqueued'

sleep interval_in_ms.to_f / 1000
end
end
end
end
end
58 changes: 58 additions & 0 deletions spec/meilisearch/index/wait_for_pending_update_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# frozen_string_literal: true

RSpec.describe MeiliSearch::Index do
before(:all) do
@documents = [
{ objectId: 123, title: 'Pride and Prejudice' },
{ objectId: 456, title: 'Le Petit Prince' },
{ objectId: 1, title: 'Alice In Wonderland' },
{ objectId: 1344, title: 'The Hobbit' },
{ objectId: 4, title: 'Harry Potter and the Half-Blood Prince' },
{ objectId: 42, title: 'The Hitchhiker\'s Guide to the Galaxy' }
]
client = MeiliSearch::Client.new($URL, $MASTER_KEY)
clear_all_indexes(client)
@index = client.create_index(uid: 'books', primaryKey: 'objectId')
end

it 'waits for pending update with default values' do
response = @index.add_documents(@documents)
update_id = response['updateId']
status = @index.wait_for_pending_update(update_id)
expect(status).to be_a(Hash)
expect(status['status']).not_to eq('enqueued')
end

it 'waits for pending update with default values after several updates' do
@index.add_documents(@documents)
@index.add_documents(@documents)
@index.add_documents(@documents)
@index.add_documents(@documents)
@index.add_documents(@documents)
response = @index.add_documents(@documents)
update_id = response['updateId']
status = @index.wait_for_pending_update(update_id)
expect(status).to be_a(Hash)
expect(status['status']).not_to eq('enqueued')
end

it 'waits for pending update with custom timeout_in_ms and raises error' do
@index.add_documents(@documents)
response = @index.add_documents(@documents)
update_id = response['updateId']
lambda {
@index.wait_for_pending_update(update_id, 1)
}.should raise_error(Timeout::Error)
end

it 'waits for pending update with custom interval_in_ms and raises error' do
@index.add_documents(@documents)
response = @index.add_documents(@documents)
update_id = response['updateId']
lambda {
Timeout.timeout(0.1) do
@index.wait_for_pending_update(update_id, 5000, 200)
end
}.should raise_error(Timeout::Error)
end
end