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

Fix asynchronous Python exception propagation in StreamingPythonExecutor/CNNScoreVariants. #7402

Merged
merged 3 commits into from
Dec 23, 2022

Conversation

cmnbroad
Copy link
Collaborator

Fix for #7401. Draft mode for now until I do more verification.

@gatk-bot
Copy link

Travis reported job failures from build 35396
Failures in the following jobs:

Test Type JDK Job ID Logs
conda openjdk8 35396.5 logs

@gileshall gileshall marked this pull request as ready for review September 29, 2021 16:29
@droazen
Copy link
Contributor

droazen commented Oct 26, 2021

@cmnbroad Is this still a draft, or is it ready for review?

@droazen droazen self-requested a review October 26, 2021 17:52
@droazen droazen self-assigned this Oct 26, 2021
@cmnbroad
Copy link
Collaborator Author

@doazen I need to re-review this myself, and see what more validation I can do. I hate to miss the release, but I won't be able to do that today.

@droazen
Copy link
Contributor

droazen commented Oct 26, 2021

@cmnbroad No problem, this isn't particularly time-sensitive. Ping me once it's ready for review.

@cmnbroad
Copy link
Collaborator Author

@droazen I thought I had updated this months ago saying that I was finished validating, and that it was ready for review, but it appears that I didn't. But anyway its ready.

@cmnbroad cmnbroad force-pushed the cn_cnn_exception branch from 7a15b2f to b61283e Compare May 2, 2022 14:11
@codecov
Copy link

codecov bot commented May 2, 2022

Codecov Report

Merging #7402 (b61283e) into master (9ae1fd8) will increase coverage by 0.002%.
The diff coverage is 84.444%.

@@               Coverage Diff               @@
##              master     #7402       +/-   ##
===============================================
+ Coverage     86.954%   86.956%   +0.002%     
- Complexity     36897     36910       +13     
===============================================
  Files           2214      2214               
  Lines         173540    173578       +38     
  Branches       18736     18736               
===============================================
+ Hits          150900    150937       +37     
- Misses         16037     16042        +5     
+ Partials        6603      6599        -4     
Impacted Files Coverage Δ
...bender/utils/runtime/AsynchronousStreamWriter.java 81.633% <0.000%> (-2.041%) ⬇️
.../walkers/vqsr/CNNScoreVariantsIntegrationTest.java 95.722% <77.778%> (-0.907%) ⬇️
.../python/StreamingPythonScriptExecutorUnitTest.java 86.957% <84.615%> (-0.763%) ⬇️
...tools/examples/ExampleStreamingPythonExecutor.java 97.222% <100.000%> (+0.556%) ⬆️
...er/utils/python/StreamingPythonScriptExecutor.java 85.345% <100.000%> (-0.125%) ⬇️
...nder/utils/io/DeleteRecursivelyOnExitPathHook.java 80.952% <0.000%> (-9.524%) ⬇️
...itute/hellbender/tools/LocalAssemblerUnitTest.java 92.448% <0.000%> (ø)
...ellbender/tools/walkers/vqsr/CNNScoreVariants.java 79.565% <0.000%> (+0.870%) ⬆️
...nder/utils/runtime/StreamingProcessController.java 69.091% <0.000%> (+1.818%) ⬆️
.../hellbender/utils/python/PythonUnitTestRunner.java 78.689% <0.000%> (+3.279%) ⬆️

Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

This looks google to me.

@lbergelson lbergelson merged commit aa05918 into master Dec 23, 2022
@lbergelson lbergelson deleted the cn_cnn_exception branch December 23, 2022 21:15
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.

4 participants