-
Notifications
You must be signed in to change notification settings - Fork 800
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
Added Response.search_after() method #1829
Conversation
88e81e3
to
0fb40d0
Compare
Nice work! it would be nice to have search_after shortcut built-in into this lib. |
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! LGTM.
@pytest.mark.asyncio | ||
async def test_search_after_no_search(async_data_client): | ||
s = AsyncSearch(index="flat-git") | ||
with raises(ValueError): |
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.
nit: using the match
parameter of pytest.raises
would help clarify what errors we're actually raising here and in the following tests
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.
Sure, I'll add that as well. Thanks!
* Added Response.search_after() method * add match clause to pytest.raises (cherry picked from commit 891ba7c)
The intention is to rebuild the change proposed in #1623 as independent pieces supporting
search_after
, point-in-time, and ultimately a better version ofscan()
that uses them.This PR implements
search_after
support in theResponse
class, also available through theSearch
class for convenience.