Fix a bug identified in batch processing

[Peter-J-Freeman]

Bug identified in batch testing, but unable to replicate. a KeyError is returned. KeyError: 'alt_strand'

[Peter-J-Freeman]

@John-F-Wagstaff I could not actually replicate the bug. This code should solve it, but I do not know if the return will be OK or whether you would want to raise.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.67%. Comparing base (66a8967) to head (725e36f).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #847      +/-   ##
===========================================
+ Coverage    76.32%   76.67%   +0.34%     
===========================================
  Files           37       37              
  Lines        13054    13253     +199     
===========================================
+ Hits          9964    10162     +198     
- Misses        3090     3091       +1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[John-F-Wagstaff]

Yes failure in that call to get_tx_exons should not be able to happen at that point, if you are getting intermittent failures that don't replicate locally then the database got overloaded and returned a duff result. We don't want to return that to the user.

Either we want to wait and retry, and fail hard if that does not work, or just default to failing at this point.

(Edit: yes that unfortunately means that return will not be OK here)

[Peter-J-Freeman]

I suspected something screwey was goin on, and that this should not happen since its a standard data structure. Shall I update this with a try again, maybe in a loop and if it takes more than a set length of time, raise??

[John-F-Wagstaff]

That might be better, it would be more user friendly, but it might be overkill for now.

Also testing database failures sounds awkward. The only way I can think of doing it requires creative testing of just this subsection, and using dummy database objects built to fail selectively.

[Peter-J-Freeman]

Can use mock for this rather than needing a dummy database. Let me see what I can cook up.

[John-F-Wagstaff]

Ah that is what I meant "dummy database objects" as in fake response giving objects that pretend to be normal database tied hgvs data providers, not actual dummy databases. I agree that the latter would be too much of a pain to set up for normal testing.

[Peter-J-Freeman]

Passing mock objects etc to make the code go down the correct pathways rather than settin up databases and real objects like hdp. It is the pathway that needs to be tested, i.e. mocking the pathway of the things that would cause the errors rather than generating real ones. It is good enough and would be, as you say, overkill to do it fully.

[John-F-Wagstaff]

Yep, exactly, but even that is a bit of a pain.

[Peter-J-Freeman]

@John-F-Wagstaff does this look like an OK solution now? I have added mock object tests and now have all but full coverage of the file. Are the retry settings before raising adequate?

[John-F-Wagstaff]

It all looks good.

Too many retries won't help. I think it is best to start low like you have, if we need to up it we might have to up delay as well, which would be non optimal if this works as-is.

[Peter-J-Freeman]

Exactly my thoughts. It's unlikely to fail once, if it does it'll likely catch the second time and work.