try to fix coordinates

[blankdots]

does this fix the coordinates ... well yes :) does it remove a lot of the unit tests ... yes
Tests will be revamped when we move to new repo with issue neicnordic/sensitive-data-archive#410

inspired by: https://github.com/neicnordic/crypt4gh/blob/master/examples/reader/main.go

• disabled logging health endpoint as it generates unnecessary logs

[codecov-commenter]

Codecov Report

Attention: 74 lines in your changes are missing coverage. Please review.

Comparison is base (5dc4fe0) 67.03% compared to head (03449f4) 62.15%.

Files	Patch %	Lines
api/sda/sda.go	0.00%	74 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
- Coverage   67.03%   62.15%   -4.89%     
==========================================
  Files          10       10              
  Lines        1611     1633      +22     
==========================================
- Hits         1080     1015      -65     
- Misses        493      583      +90     
+ Partials       38       35       -3     

Flag	Coverage Δ
unittests	62.15% <6.32%> (-4.89%)	⬇️

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.

[pontus]

Code looks good, but I'm a bit worried about the amount of unit tests cut, do all need to go? I see the added other tests ("integration"), but think we could have both.

(and now I reread the top again and is less worried.)

[pontus]

Also, I'm curious, I hope (and believe) I didn't break anything when I added random access to the crypt4gh golang.

[blankdots]

Also, I'm curious, I hope (and believe) I didn't break anything when I added random access to the crypt4gh golang.

We were using the datalistpackets, and I am sure they worked previously. I remember testing, but we don't have a test to see when and if they were actually broken

[blankdots]

marking as draft as we test more things

[blankdots]

@pontus we tried to make it work with large file chunks but we see that chunks larger than ~5Mb don't work
we pinpointed to: neicnordic/crypt4gh#83 changes as after those changes the way we did it previously worked

[blankdots]

@pontus i made it work with the example reader you created; it was my mistake trying to be fancy about how to work with it, now sda-download works with larger files as well, however the problem with the data edit list still exists in crypt4gh how we were using it previously

[jbygdell]

When this PR is merged can we archive this repo so that all future work gets done in neicnordic/sensitive-data-archive?
Also these changes needs to be moved into the sensitive-data-archive repo as well.

[blankdots]

When this PR is merged can we archive this repo so that all future work gets done in neicnordic/sensitive-data-archive?
Also these changes needs to be moved into the sensitive-data-archive repo as well.

yes, but there were some issues @aaperis reported i will see if i can replicate

[aaperis]

Hopefully, problem solved :-)

I believe the issue was that the logic for seeking the first byte where the byte range will be read was missing. As a consequence, download was always starting from byte 1. With the suggested change, sda-download, the read example from the crypt4gh repo and dd tests all agree. I also made some suggestions about the integration tests. Ideally, for the later we should use a small bam file when we migrate to the merged repo.

[blankdots]

I believe the issue was that the logic for seeking the first byte where the byte range will be read was missing.

you are right, i think i had it there, but in the confusion of me rewriting that piece many times and bad tests, i did not spot it

solved, thank you 🙇