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

Log scroll response failures #1318

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bartier
Copy link
Contributor

@bartier bartier commented Jul 7, 2020

As raised by issue #1261, when the endpoint POST /_search/scroll answers with failures indications they are not provided in the exception or log to help troubleshoot the root cause.

This PR intends to solve this adding failures indications like the example below:
image

A reason limit (5) is set to avoid the exception end up massive as pointed by @tommyzli

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link

@tommyzli tommyzli left a comment

Choose a reason for hiding this comment

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

Thanks for picking up my issue!

elasticsearch/helpers/actions.py Outdated Show resolved Hide resolved
elasticsearch/helpers/actions.py Outdated Show resolved Hide resolved
@bartier
Copy link
Contributor Author

bartier commented Jul 9, 2020

@sethmlarson Hi! If possible, could you do a code review at this PR? :)

I guess me and @tommyzli agree on the proposal code changes already.

Copy link
Contributor

@sethmlarson sethmlarson 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 write some unit tests for this functionality specifically a <=5 errors case and >5 errors case.

@bartier
Copy link
Contributor Author

bartier commented Aug 21, 2020

@sethmlarson Sure. Unfortunately I'm struggling to run the test module where I want to add my tests. I made the following steps to run and it shows a error like 'AttributeError: 'TestXXXXX' object has no attribute 'client'. I guess somehow I am not able to start ES but I'm not sure how I should make this works. If I could make this work I could continue to work doing the tests.

Any idea if I'm doing something wrong?

Notes:
I have installed docker/docker-compose and get_client from test_elasticsearch/test_server/__init__.py is raising error trying to import test_elasticsearch.local and also the fallback test.get_test_client(**kwargs) raises an error.
image

$ git clone https://github.com/bartier/elasticsearch-py.git
$ cd elasticsearch-py && git checkout log-scroll-failures
$ python3.8 -m venv venv
$ source venv/bin/activate
$ python setup.py test -s test_elasticsearch.test_server # error happens here

# Below output was truncated
======================================================================
ERROR: test_all_documents_are_read (test_elasticsearch.test_server.test_helpers.TestScan)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vitor/github/elasticsearch-py/test_elasticsearch/test_server/test_helpers.py", line 362, in test_all_documents_are_read
    self.client.bulk(bulk, refresh=True)
AttributeError: 'TestScan' object has no attribute 'client'

======================================================================
ERROR: test_clear_scroll (test_elasticsearch.test_server.test_helpers.TestScan)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vitor/github/elasticsearch-py/test_elasticsearch/test_server/test_helpers.py", line 481, in test_clear_scroll
    self.client.bulk(bulk, refresh=True)
AttributeError: 'TestScan' object has no attribute 'client'

======================================================================  

@ajosh0504
Copy link

Hi! @sethmlarson @bartier Has there been any progress on this? I am running into The same error.

@bartier
Copy link
Contributor Author

bartier commented Feb 18, 2021

Hi! @sethmlarson @bartier Has there been any progress on this? I am running into The same error.

Unfortunately it has not been progress yet. I guess maintainers should have a look at this to give a hint at least how we could run the tests, so we can make unit tests and go ahead with PRs.

@Sparkycz
Copy link
Contributor

Hi! @sethmlarson @bartier Has there been any progress on this? I am running into The same error.

Unfortunately it has not been progress yet. I guess maintainers should have a look at this to give a hint at least how we could run the tests, so we can make unit tests and go ahead with PRs.

Try to run tests over py.test command. e.g. $ py.test test_elasticsearch/test_server/test_helpers.py

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.

6 participants