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

Do not save cases/samples into the sql database and work uniquely with stats runned on d4 files on the fly #330

Merged
merged 13 commits into from
Aug 15, 2024

Conversation

northwestwitch
Copy link
Member

@northwestwitch northwestwitch commented Aug 1, 2024

THIS IS A MAJOR CHANGE,

but with the refactoring of the computation of the coverage completeness (upcoming PR), why can't we release a mayor? It's good timing.

Description

Changed

Currently chanjo2 supports saving cases and samples (including paths to local d4 files) data into the database for later use. Later use being the following endpoints:

None of the functionalities are used by us or our collaborators, so I want to simplify chanjo2 by removing them.

How to test

  • All automatic tests should run as expected
  • Deploy on stage and use a scout case to retrieve coverage report

Expected outcome

  • Case report from scout case should work as expected

Review

  • Tests executed by CR, JW
  • "Merge and deploy" approved by JW

This version is a

  • MAJOR - when you make incompatible API changes
  • MINOR - when you add functionality in a backwards compatible manner
  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

@northwestwitch northwestwitch changed the title Remove cases samples Do not save cases/samples into the sql database and work uniquely with stats runned on d4 files on the fly Aug 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.18%. Comparing base (c65c670) to head (719f1b8).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #330   +/-   ##
=======================================
  Coverage   90.18%   90.18%           
=======================================
  Files          30       30           
  Lines        1457     1457           
=======================================
  Hits         1314     1314           
  Misses        143      143           
Flag Coverage Δ
unittests 90.18% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@northwestwitch
Copy link
Member Author

northwestwitch commented Aug 14, 2024

Hi @Jakob37, I was wondering if you have some time to give this PR a review? It's not adding any new feature, only removing stuff we are not using (see PR description) thus simplifying the whole thing. Thanks!

@Jakob37
Copy link
Collaborator

Jakob37 commented Aug 14, 2024

Hi @Jakob37, I was wondering if you have some time to give this PR a review? It's not adding any new feature, only removing stuff we are not using (see PR description) thus simplifying the whole thing. Thanks!

Yes, I can take a look before the end of this week!

@Jakob37
Copy link
Collaborator

Jakob37 commented Aug 15, 2024

I ran this locally together with the Scout stack. The latest main branch of Scout.

I needed to mount in the Scout demo data folder to get it working - before that Chanjo2 couldn't access the d4 file. But that is probably as expected.

Looks good!

new_chanjo2

@Jakob37
Copy link
Collaborator

Jakob37 commented Aug 15, 2024

I eyed through the code as well. Didn't find anything to comment on there. This seems to simplify the code a lot, nice 😄

@northwestwitch
Copy link
Member Author

I eyed through the code as well. Didn't find anything to comment on there. This seems to simplify the code a lot, nice 😄

Thank you! Let's go for this and a new major release when we have the new faster completeness stats in place!

Copy link

@northwestwitch northwestwitch merged commit bd57ae0 into main Aug 15, 2024
7 checks passed
@northwestwitch northwestwitch deleted the remove_cases_samples branch September 26, 2024 12:32
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.

Simplify app by not saving cases and samples into the database
3 participants