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

DSEGOG-317 Bug fix for updating records #123

Merged
merged 3 commits into from
May 29, 2024

Conversation

MRichards99
Copy link
Collaborator

This PR fixes a bug I found when records are updated but the images/waveforms aren't present on Echo. The channels were skipped because the database says the channel exists but this doesn't always mean the data itself exists on Echo. An additional check has been added for image and waveform channels to see if it's actually on Echo or not. If the file isn't present on Echo, the channel is ingested (meaning the file will end up on Echo).

@MRichards99
Copy link
Collaborator Author

Safety job on CI is failing which will be fixed when #122 is merged

@codecov-commenter
Copy link

codecov-commenter commented May 14, 2024

Codecov Report

Attention: Patch coverage is 84.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 94.91%. Comparing base (0063ced) to head (a5343c8).
Report is 8 commits behind head on main.

Files Patch % Lines
...api/src/records/ingestion/partial_import_checks.py 84.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   94.99%   94.91%   -0.09%     
==========================================
  Files          50       50              
  Lines        2837     2871      +34     
  Branches      297      300       +3     
==========================================
+ Hits         2695     2725      +30     
- Misses        105      108       +3     
- Partials       37       38       +1     

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

Copy link
Contributor

@patrick-austin patrick-austin left a comment

Choose a reason for hiding this comment

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

I don't think this will work for non-Image/Waveform channels (but have only looked at this in the GH web UI, not tried to actually run it so might be wrong...)

Copy link
Contributor

@patrick-austin patrick-austin left a comment

Choose a reason for hiding this comment

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

Change looks good to me, glad to hear it works with a image/waveform-less import.

@MRichards99 MRichards99 merged commit 117f169 into main May 29, 2024
9 checks passed
@MRichards99 MRichards99 deleted the DSEGOG-317-missing-partial-import branch May 29, 2024 13:24
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.

None yet

3 participants