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

MapBuilder retry_failed bug #111

Merged
merged 2 commits into from
May 20, 2020
Merged

MapBuilder retry_failed bug #111

merged 2 commits into from
May 20, 2020

Conversation

acrutt
Copy link
Contributor

@acrutt acrutt commented Mar 16, 2020

Bug fix so that retry_failed=True argument of MapBuilder does result in attempting to process failed docs in the target store.

acrutt and others added 2 commits March 16, 2020 15:21
Syncing changes from upstream
…in attempting to processing failed docs in the target store.
@codecov
Copy link

codecov bot commented Mar 16, 2020

Codecov Report

Merging #111 into master will decrease coverage by 0.7%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #111      +/-   ##
==========================================
- Coverage   81.57%   80.87%   -0.71%     
==========================================
  Files          21       21              
  Lines        1563     1563              
==========================================
- Hits         1275     1264      -11     
- Misses        288      299      +11
Impacted Files Coverage Δ
src/maggma/builders/map_builder.py 91.2% <ø> (ø) ⬆️
src/maggma/stores/mongolike.py 88.48% <0%> (-5.76%) ⬇️

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 3b8bd35...d343075. Read the comment docs.

@shyamd
Copy link
Contributor

shyamd commented Mar 17, 2020

Can you add a test for this so we don't miss it in the future?

@acrutt
Copy link
Contributor Author

acrutt commented Mar 17, 2020

I can work on a test but will need more instructions/guidance of what needs to be done since I haven't made one before.

@shyamd
Copy link
Contributor

shyamd commented Mar 18, 2020

You might actually write tests for your ProjectionBuilder. You can model it off the copybuilder or group builder tests. The idea is to make a mock data set, use memory stores to run the builder and make sure all the different functionality works. Feel free to write up something and push the changes here. I can comment on what to do after that.

https://github.com/materialsproject/maggma/blob/master/tests/builders/test_group_builder.py

@shyamd shyamd merged commit 5de5a5a into materialsproject:master May 20, 2020
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.

2 participants