-
Notifications
You must be signed in to change notification settings - Fork 48
Implement wait_for_pending_update method #43
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
Conversation
- Implement wait_for_pending_update method - Add tests for wait_for_pending_update
850adaa
to
d013e2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!!
lib/meilisearch/index.rb
Outdated
|
||
module MeiliSearch | ||
class Index < HTTPRequest | ||
class Index < HTTPRequest # rubocop:disable Metrics/ClassLength |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
lib/meilisearch/index.rb
Outdated
@@ -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) |
There was a problem hiding this comment.
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?
lib/meilisearch/index.rb
Outdated
@@ -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) | |||
Timeout.timeout(timeout_in_ms.to_f / 1000) do |
There was a problem hiding this comment.
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:
- custom error: https://github.com/meilisearch/meilisearch-php/blob/master/src/Exceptions/TimeOutException.php
- the method: https://github.com/meilisearch/meilisearch-php/blob/13cc34d4ee19b7bbf674d8f9af57545c8d6338e9/src/Index.php#L94
(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 ^^)
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!!! 👌
Here is a new ruby developer in the team 😉
Merging...
* Wait for pending update method - Implement wait_for_pending_update method - Add tests for wait_for_pending_update * Lint auto-generated file * Custom error
Based on this discussion, SDKs must provide a method that waits synchronously for an update to be processed by MeiliSearch.
Parameters:
update_id
: id of the update to be waitedOptional parameters:
timeout_in_ms
max number of millisecond this method should wait before rising aTimeoutError
(default=2000ms)interval_in_ms
number of millisecond to set an interval of time this method should wait (sleep) between requests (default=10ms)tests
wait_for_pending_update
method in theIndex
classCloses #25