un-indenting head-safe check to apply for cached files too

[eas342]

Un-indenting the head-safe boolean check to apply to both fixing incomplete cached files and new files. This appears to fix this issue:
#3231

However, I do not understand what this head_safe keyword does, so please check that this does not break the intended functionality or flow.

[codecov]

Codecov Report

Attention: Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.32%. Comparing base (1f45a44) to head (b599c09).
Report is 87 commits behind head on main.

Files with missing lines	Patch %	Lines
astroquery/query.py	84.21%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3232      +/-   ##
==========================================
+ Coverage   69.08%   69.32%   +0.23%     
==========================================
  Files         232      232              
  Lines       19592    19661      +69     
==========================================
+ Hits        13536    13630      +94     
+ Misses       6056     6031      -25     

☔ View full report in Codecov by Sentry.
📢 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.

[snbianco]

Hi again @eas342, thanks for this PR! This change is to a file in the main Astroquery directory and not part of any particular module, so it will impact more than just the MAST module. I would rather we change the MAST module directly so that it knows to continue incomplete file downloads. See my comment on the issue here: #3231 (comment)

[snbianco]

See other comment!

[keflavich]

Unindenting this block is not the correct behavior; it will overwrite the continuation block above, which would result in the download continuation failing - it will download the whole file and append it to whatever was partly completed.

However, this same code block (the if head_safe: response = request(...) block) needs to be repeated above, in the other location open_mode='wb'

[eas342]

Thank you @keflavich ,

Unfortunately, the change you made does not seem to pass #3231

import os
from astropy.io import fits
from astroquery.mast import Observations
fileN = 'jw01185103001_02102_00001-seg001_nrcalong_rate.fits'
#os.remove(fileN)
Observations.download_file('mast:jwst/product/' + fileN)
with open(fileN, 'r+') as f:
    f.seek(1000)
    f.truncate()
Observations.download_file('mast:jwst/product/' + fileN)
tmpHDU = fits.open(fileN)

WARNING: Found cached file jw01185103001_02102_00001-seg001_nrcalong_rate.fits with size 0 that is different from expected size 83520 [astroquery.query]
Downloading URL https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:jwst/product/jw01185103001_02102_00001-seg001_nrcalong_rate.fits to jw01185103001_02102_00001-seg001_nrcalong_rate.fits ...
|==========================================| 83k/ 83k (100.00%) 0s
WARNING: Found cached file jw01185103001_02102_00001-seg001_nrcalong_rate.fits with size 1000 that is different from expected size 83520 [astroquery.query]
Downloading URL https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:jwst/product/jw01185103001_02102_00001-seg001_nrcalong_rate.fits to jw01185103001_02102_00001-seg001_nrcalong_rate.fits ...
|==========================================| 83k/ 83k (100.00%) 0s
OSError: Empty or corrupt FITS file

[keflavich]

Thanks @eas342. Indeed, there was another corner case we hadn't checked. The latest commit catches it.

[eas342]

That works, super! Thank you @keflavich

[bsipocz]

@eas342 @keflavich - I'm not sure how to test this in CI, etc. But if you are certain it covers all the cases, and add a changelog, it can still go in the next release ~tomorrow.
But if there is no rush or there is any uncertainty, then it should sit the release out and get it merged after it's out to have it in main for a little longer.

[keflavich]

let's add tests; i think i can turn @eas342 's mwe into a test easily enough. so delay merging

[keflavich]

@bsipocz this is ready for final review