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

Fix missing docs.count #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

james-stead
Copy link

Our Elastic cluster has a number of indexes that don't return this value. The indexes do appear to be empty as well from looking at the indexes in elastic. This is the for example is what is in item:
{'health': 'green', 'status': 'open', 'index': '', 'uuid': 't_w0eDA9SXS8CGYhYtXhfg', 'pri': '1', 'rep': '1', 'docs.count': None, 'docs.deleted': None, 'store.size': None, 'pri.store.size': None}

Our Elastic cluster has a number of indexes that don't return this value.  The indexes do appear to be empty as well from looking at the indexes in elastic.  This is the for example is what is in item:
{'health': 'green', 'status': 'open', 'index': '<my index>', 'uuid': 't_w0eDA9SXS8CGYhYtXhfg', 'pri': '1', 'rep': '1', 'docs.count': None, 'docs.deleted': None, 'store.size': None, 'pri.store.size': None}
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2022

Codecov Report

Merging #91 (aae46d0) into master (7e48e57) will decrease coverage by 0.28%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master      #91      +/-   ##
==========================================
- Coverage   94.40%   94.12%   -0.29%     
==========================================
  Files          15       15              
  Lines         966      970       +4     
==========================================
+ Hits          912      913       +1     
- Misses         54       57       +3     
Flag Coverage Δ
python 94.12% <57.14%> (-0.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
es/elastic/api.py 84.61% <57.14%> (-2.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e48e57...aae46d0. Read the comment docs.

@dpgaspar
Copy link
Member

dpgaspar commented Apr 20, 2022

Hi @james-stead,

Thank you for your contribution, looks good to me.

Can you please update your branch with the current master. I've just merged a PR that will fix CI lint step.

More completeness and stability would be super great if you could add a test to verify that an index without a count will behave has expected

@dpgaspar dpgaspar self-requested a review April 20, 2022 08:56
if int(item["docs.count"]) == 0:
is_empty = True
break
except Exception as ex:
Copy link
Member

Choose a reason for hiding this comment

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

too broad exception, replace it by TypeError. Also it seems less clear what can be expected from the ES index.

what do you think about:

if item["docs.count"] is None or item["docs.count"] in (0, "0")

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.

3 participants