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

Check in parser if the source has stable IDs and if not, delete the existing cases #1950

Merged
merged 5 commits into from
Jun 18, 2021

Conversation

iamleeg
Copy link
Contributor

@iamleeg iamleeg commented Jun 18, 2021

Mostly putting this up as draft now as it's the easiest way to get the dockerised tests to run, which are the only way to test the function I need to change.

#1921

@iamleeg iamleeg marked this pull request as ready for review June 18, 2021 12:39
@abhidg
Copy link
Contributor

abhidg commented Jun 18, 2021

Looks good, other than the default for hasStableIdentifiers which should be False as that's the case for majority. If a source actually has stable identifiers with hasStableIdentifiers, we are going to ingest the entire dataset needlessly, but it won't have incorrect data.

@codecov-commenter
Copy link

Codecov Report

Merging #1950 (20198ab) into main (0bc1a9e) will decrease coverage by 10.38%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1950       +/-   ##
===========================================
- Coverage   66.63%   56.25%   -10.39%     
===========================================
  Files         132       76       -56     
  Lines        4466     2622     -1844     
  Branches     1174      837      -337     
===========================================
- Hits         2976     1475     -1501     
+ Misses       1490     1147      -343     
Impacted Files Coverage Δ
...-service/ui/src/components/AutomatedSourceForm.tsx 44.00% <ø> (ø)
.../curator-service/ui/src/components/SourceTable.tsx 62.58% <100.00%> (ø)
...fication/curator-service/api/src/model/schedule.ts
data-serving/data-service/src/model/travel.ts
...rving/data-service/src/geocoding/remoteGeocoder.ts
...a-serving/data-service/src/model/travel-history.ts
...rification/curator-service/api/src/model/origin.ts
data-serving/data-service/src/util/search.ts
...ion/curator-service/api/src/controllers/uploads.ts
...-serving/data-service/src/model/genome-sequence.ts
... and 48 more

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 0bc1a9e...20198ab. Read the comment docs.

@@ -41094,6 +40963,20 @@
"whatwg-fetch": "^3.0.0"
}
},
"react-beautiful-dnd": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this part of another PR? Seems to be from the highlighting search terms work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You know what? I don't know. I didn't explicitly put this here, I ran npm i to ensure the deps were up to date and I committed the change to the lock file. I can't find react-beautiful-dnd in the package.json so I can revert this file if you think it's wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, so the highlighting work seems to be already in main, which would explain this. I'm fine with keeping this change.

verification/curator-service/ui/src/components/ViewCase.tsx
30:import Highlighter from 'react-highlight-words'

@@ -41094,6 +40963,20 @@
"whatwg-fetch": "^3.0.0"
}
},
"react-beautiful-dnd": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Right, so the highlighting work seems to be already in main, which would explain this. I'm fine with keeping this change.

verification/curator-service/ui/src/components/ViewCase.tsx
30:import Highlighter from 'react-highlight-words'

@iamleeg iamleeg merged commit 75a5581 into main Jun 18, 2021
@iamleeg iamleeg deleted the 1921_parser_lib_ingesting_unstable_id_sources branch June 18, 2021 16:52
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