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 #79

Merged
merged 6 commits into from
May 27, 2020

Conversation

eskombro
Copy link
Member

@eskombro eskombro commented May 26, 2020

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 waited

Optional parameters:

  • timeout_in_ms max number of millisecond this method should wait before rising a TimeoutError (default=2000ms)
  • interval_in_ms number of millisecond to set an interval of time this method should wait (sleep) between requests (default=10ms)

tests

  • Added wait_for_pending_update method in the Index class
  • Added tests for default values, test for timeout and test for interval.

Closes #64

@eskombro eskombro self-assigned this May 26, 2020
@eskombro eskombro force-pushed the wait_for_pending_update_method branch 6 times, most recently from e26582a to b4ede03 Compare May 26, 2020 21:02
@eskombro eskombro force-pushed the wait_for_pending_update_method branch from b4ede03 to c502a34 Compare May 26, 2020 21:04
@eskombro eskombro requested review from curquiza and bidoubiwa May 26, 2020 21:05
@eskombro eskombro mentioned this pull request May 27, 2020
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Sorry for my shitty comments, so bad that the python linter does not see that. I feel more and more disappointed by this linter 😂

But there is still one important comment 😬

meilisearch/index.py Outdated Show resolved Hide resolved
meilisearch/index.py Outdated Show resolved Hide resolved
meilisearch/index.py Outdated Show resolved Hide resolved
meilisearch/index.py Outdated Show resolved Hide resolved
meilisearch/index.py Outdated Show resolved Hide resolved
meilisearch/index.py Outdated Show resolved Hide resolved
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

Could you add a test with interval = 0 ?

return get_update
sleep(interval_in_ms/1000)
time_delta = datetime.now() - start_time
elapsed_time = time_delta.seconds * 1000 + time_delta.microseconds / 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this line do?

Copy link
Member Author

Choose a reason for hiding this comment

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

It uses the timedelta object to calculate the number of milliseconds since the moment the user called the wait_for_pending_update method. time_delta is an instance of a timedelta object, this just converts it to an int in ms

Co-authored-by: Clémentine Urquizar <clementine@meilisearch.com>
Copy link
Member Author

@eskombro eskombro left a comment

Choose a reason for hiding this comment

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

Could you add a test with interval = 0 ?

@bidoubiwa whats would seem like a logical behaviour for you in that test? I don't really know what I could check other than if the function works or not

return get_update
sleep(interval_in_ms/1000)
time_delta = datetime.now() - start_time
elapsed_time = time_delta.seconds * 1000 + time_delta.microseconds / 1000
Copy link
Member Author

Choose a reason for hiding this comment

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

It uses the timedelta object to calculate the number of milliseconds since the moment the user called the wait_for_pending_update method. time_delta is an instance of a timedelta object, this just converts it to an int in ms

@bidoubiwa
Copy link
Contributor

@eskombro The only behavior is indeed for me that it should not crash. Since there are divisions going on I always prefer if the zero values are tested.

@eskombro eskombro requested review from curquiza and bidoubiwa May 27, 2020 11:46
@eskombro eskombro merged commit 341fc74 into master May 27, 2020
@eskombro eskombro deleted the wait_for_pending_update_method branch May 27, 2020 12:28
@eskombro eskombro restored the wait_for_pending_update_method branch May 27, 2020 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wait_for_pending_update method
3 participants