Skip to content

Commit

Permalink
Merge pull request #2224 from andamian/almafix
Browse files Browse the repository at this point in the history 
ALMA integration tests fix
  • Loading branch information
@bsipocz
bsipocz authored Nov 23, 2021
2 parents cbec82e + b2bd97c commit e8489d3
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 137 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ sdss
Infrastructure, Utility and Other Changes and Additions
-------------------------------------------------------

- Adding ``--alma-site`` pytest option for testing to have a control over
which specific site to test. [#2224]


0.4.4 (2021-11-17)
Expand Down
7 changes: 0 additions & 7 deletions astroquery/alma/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
'https://almascience.nrao.edu',
'https://almascience.nao.ac.jp']

_test_url_list = ['https://beta.cadc-ccda.hia-ha.nrc-cnrc.gc.ca']

auth_urls = ['asa.alma.cl', 'rh-cas.alma.cl']


Expand All @@ -27,11 +25,6 @@ class Conf(_config.ConfigNamespace):
_url_list,
'The ALMA Archive mirror to use.')

test_archive_url = _config.ConfigItem(
_test_url_list,
'ALMA Archive Test Mirrors (temporary)'
)

auth_url = _config.ConfigItem(
auth_urls,
'ALMA Central Authentication Service URLs'
Expand Down
39 changes: 19 additions & 20 deletions astroquery/alma/core.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
},
'Energy': {
'Frequency (GHz)': ['frequency', 'frequency', _gen_numeric_sql],
'Bandwidth (GHz)': ['bandwidth', 'bandwidth', _gen_numeric_sql],
'Bandwidth (Hz)': ['bandwidth', 'bandwidth', _gen_numeric_sql],
'Spectral resolution (KHz)': ['spectral_resolution',
'em_resolution', _gen_spec_res_sql],
'Band': ['band_list', 'band_list', _gen_band_list_sql]
Expand Down Expand Up @@ -677,7 +677,7 @@ def _HEADER_data_size(self, files):
return data_sizes, totalsize.to(u.GB)

def download_files(self, files, savedir=None, cache=True,
continuation=True, skip_unauthorized=True):
continuation=True, skip_unauthorized=True,):
"""
Given a list of file URLs, download them
Expand Down Expand Up @@ -708,22 +708,21 @@ def download_files(self, files, savedir=None, cache=True,
downloaded_files = []
if savedir is None:
savedir = self.cache_location
for fileLink in unique(files):
log.debug("Downloading {0} to {1}".format(fileLink, savedir))
for file_link in unique(files):
log.debug("Downloading {0} to {1}".format(file_link, savedir))
try:
check_filename = self._request('HEAD', fileLink, auth=auth,
stream=True)
check_filename = self._request('HEAD', file_link, auth=auth)
check_filename.raise_for_status()
except requests.HTTPError as ex:
if ex.response.status_code == 401:
if skip_unauthorized:
log.info("Access denied to {url}. Skipping to"
" next file".format(url=fileLink))
" next file".format(url=file_link))
continue
else:
raise(ex)

if 'text/html' in check_filename.headers['Content-Type']:
if 'text/html' in check_filename.headers.get('Content-Type', ''):
raise ValueError("Bad query. This can happen if you "
"attempt to download proprietary "
"data when not logged in")
Expand All @@ -732,7 +731,7 @@ def download_files(self, files, savedir=None, cache=True,
filename = re.search("filename=(.*)",
check_filename.headers['Content-Disposition']).groups()[0]
except KeyError:
log.info(f"Unable to find filename for {fileLink} "
log.info(f"Unable to find filename for {file_link} "
"(missing Content-Disposition in header). "
"Skipping to next file.")
continue
Expand All @@ -742,42 +741,42 @@ def download_files(self, files, savedir=None, cache=True,
filename)

try:
self._download_file(fileLink,
self._download_file(file_link,
filename,
timeout=self.TIMEOUT,
auth=auth,
cache=cache,
method='GET',
head_safe=True,
head_safe=False,
continuation=continuation)

downloaded_files.append(filename)
except requests.HTTPError as ex:
if ex.response.status_code == 401:
if skip_unauthorized:
log.info("Access denied to {url}. Skipping to"
" next file".format(url=fileLink))
" next file".format(url=file_link))
continue
else:
raise(ex)
elif ex.response.status_code == 403:
log.error("Access denied to {url}".format(url=fileLink))
if 'dataPortal' in fileLink and 'sso' not in fileLink:
log.error("Access denied to {url}".format(url=file_link))
if 'dataPortal' in file_link and 'sso' not in file_link:
log.error("The URL may be incorrect. Try using "
"{0} instead of {1}"
.format(fileLink.replace('dataPortal/',
'dataPortal/sso/'),
fileLink))
.format(file_link.replace('dataPortal/',
'dataPortal/sso/'),
file_link))
raise ex
elif ex.response.status_code == 500:
# empirically, this works the second time most of the time...
self._download_file(fileLink,
self._download_file(file_link,
filename,
timeout=self.TIMEOUT,
auth=auth,
cache=cache,
method='GET',
head_safe=True,
head_safe=False,
continuation=continuation)

downloaded_files.append(filename)
Expand Down Expand Up @@ -1107,7 +1106,7 @@ def download_and_extract_files(self, urls, delete=True, regex=r'.*\.fits$',
expanded_files += [x for x in files['access_url'] if
filere.match(x.split('/')[-1])]
else:
tar_files.append(tar_file)
tar_files.append(url)

try:
# get the tar files
Expand Down
31 changes: 31 additions & 0 deletions astroquery/alma/tests/test_alma.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,3 +410,34 @@ def test_galactic_query():
radius=1*u.deg, get_query_payload=True)

assert result['ra_dec'] == SkyCoord(0*u.deg, 0*u.deg, frame='galactic').icrs.to_string() + ", 1.0"


def test_download_files():
def _requests_mock(method, url, **kwargs):
response = Mock()
response.headers = {
'Content-Disposition': 'attachment; '
'filename={}'.format(url.split('/')[-1])}
return response

def _download_file_mock(url, file_name, **kwargs):
return file_name
alma = Alma()
alma._request = Mock(side_effect=_requests_mock)
alma._download_file = Mock(side_effect=_download_file_mock)
downloaded_files = alma.download_files(['https://location/file1'])
assert len(downloaded_files) == 1
assert downloaded_files[0].endswith('file1')

alma._request.reset_mock()
alma._download_file.reset_mock()
downloaded_files = alma.download_files(['https://location/file1',
'https://location/file2'])
assert len(downloaded_files) == 2

# error cases
alma._request = Mock()
# no Content-Disposition results in no downloaded file
alma._request.return_value = Mock(headers={})
result = alma.download_files(['https://location/file1'])
assert not result
Loading

0 comments on commit e8489d3

Please sign in to comment.