Skip to content

Conversation

@yuki-mt
Copy link
Member

@yuki-mt yuki-mt commented Mar 27, 2019

What is this PR for?

prevent OOM with large List[EvaluateResultDetail] data

This PR includes

  • evaluate method returns generator of EvaluateResultDetail (and lastly return EvaluateResult)
  • save EvaluateResultDetail one by one
  • load EvaluateResultDetail one by one

What type of PR is it?

Feature/Bugfix/....

What is the issue?

fixed #25

How should this be tested?

python -m unittest

@yuki-mt yuki-mt requested a review from keigohtr March 27, 2019 12:24
@CLAassistant
Copy link

CLAassistant commented Mar 27, 2019

CLA assistant check
All committers have signed the CLA.

@yuki-mt yuki-mt self-assigned this Mar 27, 2019
@yuki-mt yuki-mt force-pushed the feature/prevent_OOM_due_to_evaluation_data branch from bbb4bd0 to 505d8e6 Compare March 27, 2019 12:27
@codecov-io
Copy link

codecov-io commented Mar 27, 2019

Codecov Report

Merging #44 into master will decrease coverage by 0.45%.
The diff coverage is 87.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   88.06%   87.61%   -0.46%     
==========================================
  Files          15       15              
  Lines         863      872       +9     
==========================================
+ Hits          760      764       +4     
- Misses        103      108       +5
Impacted Files Coverage Δ
rekcurd/data_servers/__init__.py 96.29% <100%> (+0.17%) ⬆️
rekcurd/core/rekcurd_worker.py 76.47% <100%> (ø) ⬆️
rekcurd/core/rekcurd_dashboard_servicer.py 93.04% <70.58%> (-4.26%) ⬇️

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 d3f7c4d...505d8e6. Read the comment docs.

Copy link
Member

@keigohtr keigohtr left a comment

Choose a reason for hiding this comment

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

Almost LGTM. Great work with you!
I made some questions, could you give me a feedback?

Copy link
Member

@keigohtr keigohtr left a comment

Choose a reason for hiding this comment

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

LGTM

@yuki-mt yuki-mt merged commit e9cf668 into master Mar 28, 2019
@yuki-mt yuki-mt deleted the feature/prevent_OOM_due_to_evaluation_data branch March 28, 2019 05:44
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.

Large Evaluation data may cause OOM error

5 participants