Retry results download if connection times out#2440
Conversation
aeisenberg
left a comment
There was a problem hiding this comment.
Thanks for your contribution. This looks reasonable to me. One small suggestion.
extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts
Outdated
Show resolved
Hide resolved
aeisenberg
left a comment
There was a problem hiding this comment.
Apologies. I just thought of this. We should log whenever we do a retry.
My code isn't exactly correct since there's no associated import statement.
extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts
Outdated
Show resolved
Hide resolved
|
Failing tests are flakes. I'll re-run. |
koesie10
left a comment
There was a problem hiding this comment.
Thanks for adding this! Unfortunately, there is some extra work required to make this work when the connection drops when some parts have already been downloaded. When downloading, we only append to the ZIP file:
If we restart the download without truncating this file, a corrupt ZIP file will be written. Can you add await rm(zipFilePath, { force: true }); (with rm coming from fs-extra) in the download method of the VariantAnalysisResultsManager just before starting the fetch?
extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts
Outdated
Show resolved
Hide resolved
|
Good to be merged? |
aeisenberg
left a comment
There was a problem hiding this comment.
Looks good! Thanks for reminding me to take another peak.
Even with a good internet connection I sometimes have seen the error "failed to download results". When doing a MRVA on a 1000 top projects this results in that either you admit your run has incomplete results or have to rerun whole 1000 repositories query run, which is time consuming.
I have noticed that the url in the error message is still valid and I'm able to download the zip manually.
The change makes up to 3 retry attempts in a specific cases like timeout.
Checklist
ready-for-doc-reviewlabel there.