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

Added fix for key error because of missing 'hits' key. #616

Conversation

Djcarrillo6
Copy link
Contributor

@Djcarrillo6 Djcarrillo6 commented Nov 25, 2023

Description

Adds graceful/defensive code for handling case of missing "hits" key in scan response.

Issues Resolved

Closes #313

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Nov 25, 2023

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (6f26eb3) 71.98% compared to head (fad9307) 72.12%.
Report is 2 commits behind head on main.

Files Patch % Lines
opensearchpy/_async/helpers/actions.py 0.00% 7 Missing ⚠️
opensearchpy/helpers/actions.py 14.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   71.98%   72.12%   +0.14%     
==========================================
  Files          89       89              
  Lines        7935     7939       +4     
==========================================
+ Hits         5712     5726      +14     
+ Misses       2223     2213      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Djcarrillo6 Djcarrillo6 force-pushed the fix/issue-313/graceful-empty-scan-result-handling branch from 71137f2 to 4df886c Compare November 25, 2023 21:27
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

AFAIK tests should go next to other scroll tests, so in test_actions.py?

Btw, tests aren't very well organized, my rule of thumb is that for every file like actions.py there should be one test_actions.py and if actions is a kitchen sink of many things, i can be split up into a folder called actions with files such as scroll.py, and tests organized similarly. So whatever gets us closer to that is A-OK.

CHANGELOG.md Outdated Show resolved Hide resolved
@Djcarrillo6 Djcarrillo6 force-pushed the fix/issue-313/graceful-empty-scan-result-handling branch 2 times, most recently from 63aa1eb to 64f2f6b Compare November 26, 2023 20:27
Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Updated CHANGELOG.md.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

nox formatting applied.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Added new unit test for actions scan function.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Added type hints & nox formatting.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Added fix to async scan function & added matching unit tests for async.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>
@Djcarrillo6 Djcarrillo6 force-pushed the fix/issue-313/graceful-empty-scan-result-handling branch from 64f2f6b to fad9307 Compare December 1, 2023 22:16
@dblock dblock merged commit db61b59 into opensearch-project:main Dec 4, 2023
53 of 54 checks passed
roma2023 pushed a commit to roma2023/opensearch-py that referenced this pull request Dec 28, 2023
…oject#616)

Updated CHANGELOG.md.

nox formatting applied.

Added new unit test for actions scan function.

Added type hints & nox formatting.

Added fix to async scan function & added matching unit tests for async.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>
Signed-off-by: roma2023 <romasaparhan19@gmail.com>
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.

[BUG] scan throws KeyError: 'hits' when no results are found
3 participants