Repository: rucio/rucio
Issue: Download client does not filter out tape sources

Motivation
----------

This happens also with an old version of the Rucio client. Either the issue has gone unnoticed for a very long time or something changed server-side.

```
$ rucio --version
rucio 1.25.5
$ rucio list-file-replicas data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1
+------------+---------------------------------------------------------------------------+------------+-----------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| SCOPE      | NAME                                                                      | FILESIZE   | ADLER32   | RSE: REPLICA                                                                                                                                                                                                                                                            |
|------------+---------------------------------------------------------------------------+------------+-----------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| data15_cos | data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1 | 444.412 kB | 062e8eda  | CERN-PROD_RAW: root://eosctaatlas.cern.ch:1094//eos/ctaatlas/archive/grid/atlas/rucio/raw//data15_cos/physics_CosmicCalo/00266048/data15_cos.00266048.physics_CosmicCalo.merge.RAW/data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1_1432720937 |
| data15_cos | data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1 | 444.412 kB | 062e8eda  | SARA-MATRIX_DATATAPE: srm://srm.grid.sara.nl:8443/srm/managerv2?SFN=/pnfs/grid.sara.nl/data/atlas/atlasdatatape/data15_cos/RAW/other/data15_cos.00266048.physics_CosmicCalo.merge.RAW/data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1         |
+------------+---------------------------------------------------------------------------+------------+-----------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
$ rucio --verbose download data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1
2021-12-21 09:40:14,095	INFO	Processing 1 item(s) for input
2021-12-21 09:40:14,095	DEBUG	downloadclient.py	num_unmerged_items=1; num_dids=1; num_merged_items=1
2021-12-21 09:40:14,095	INFO	Getting sources of DIDs
2021-12-21 09:40:14,129	DEBUG	downloadclient.py	schemes: None
2021-12-21 09:40:14,130	DEBUG	downloadclient.py	rse_expression: None
2021-12-21 09:40:14,130	DEBUG	downloadclient.py	num DIDs for list_replicas call: 1
2021-12-21 09:40:14,213	DEBUG	downloadclient.py	num resolved files: 1
2021-12-21 09:40:14,233	DEBUG	downloadclient.py	"unzip -v" returned with exitcode 0
2021-12-21 09:40:14,250	DEBUG	downloadclient.py	"tar --version" returned with exitcode 0
2021-12-21 09:40:14,250	DEBUG	downloadclient.py	Queueing file: data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1
2021-12-21 09:40:14,250	DEBUG	downloadclient.py	real parents: set([])
2021-12-21 09:40:14,251	DEBUG	downloadclient.py	options: {'data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1': {'ignore_checksum': False, 'transfer_speed_timeout': 500.0, 'transfer_timeout': None, 'destinations': set([('.', False)])}}
2021-12-21 09:40:14,253	DEBUG	downloadclient.py	Prepared sources: num_sources=2/2; num_non_cea_sources=2; num_cea_ids=0
2021-12-21 09:40:14,254	INFO	Using main thread to download 1 file(s)
2021-12-21 09:40:14,254	DEBUG	downloadclient.py	Start processing queued downloads
2021-12-21 09:40:14,254	INFO	Preparing download of data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1
2021-12-21 09:40:14,305	INFO	Trying to download with root and timeout of 60s from CERN-PROD_RAW: data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1
…
```

Here’s the bit that I don’t understand:
https://github.com/rucio/rucio/blob/fa92dc8638a663aef0c54039623cbf0a48acd320/lib/rucio/client/downloadclient.py#L1153-L1162

`src` is a dictionary and `tape_rses` is a list of strings. How would a comparison work?

```
>>> metalink = client.list_replicas([{'scope': 'data15_cos', 'name': 'data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1'}], metalink=True)
>>> file_items = parse_replicas_from_string(metalink)
>>> pprint.pprint(file_items)
[{'adler32': '062e8eda',
  'bytes': 444412,
  'did': 'data15_cos:data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1',
  'md5': None,
  'parent_dids': set(),
  'sources': [{'client_extract': False,
               'domain': 'wan',
               'pfn': 'root://eosctaatlas.cern.ch:1094//eos/ctaatlas/archive/grid/atlas/rucio/raw//data15_cos/physics_CosmicCalo/00266048/data15_cos.00266048.physics_CosmicCalo.merge.RAW/data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1_1432720937',
               'priority': '1',
               'rse': 'CERN-PROD_RAW'},
              {'client_extract': False,
               'domain': 'wan',
               'pfn': 'srm://srm.grid.sara.nl:8443/srm/managerv2?SFN=/pnfs/grid.sara.nl/data/atlas/atlasdatatape/data15_cos/RAW/other/data15_cos.00266048.physics_CosmicCalo.merge.RAW/data15_cos.00266048.physics_CosmicCalo.merge.RAW._lb0002._SFO-ALL._0001.1',
               'priority': '2',
               'rse': 'SARA-MATRIX_DATATAPE'}]}]

```

Modification
------------




Changed files:
lib/rucio/tests/test_download.py, lib/rucio/client/downloadclient.py

diff --git a/lib/rucio/client/downloadclient.py b/lib/rucio/client/downloadclient.py
index 8dd1fd7bb..ec1eec3cc 100644
--- a/lib/rucio/client/downloadclient.py
+++ b/lib/rucio/client/downloadclient.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Copyright 2018-2021 CERN
+# Copyright 2018-2022 CERN
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -33,7 +33,7 @@
 # - Rakshita Varadarajan <rakshitajps@gmail.com>, 2021
 # - David Población Criado <david.poblacion.criado@cern.ch>, 2021
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2021
-# - Joel Dierkes <joel.dierkes@cern.ch>, 2021
+# - Joel Dierkes <joel.dierkes@cern.ch>, 2021-2022
 
 from __future__ import division
 
@@ -1205,7 +1205,7 @@ class DownloadClient:
                 for file_item in file_items:
                     unfiltered_sources = copy.copy(file_item['sources'])
                     for src in unfiltered_sources:
-                        if src in tape_rses:
+                        if src['rse'] in tape_rses:
                             file_item['sources'].remove(src)
                     if unfiltered_sources and not file_item['sources']:
                         logger(logging.WARNING, 'The requested DID {} only has replicas on tape. Direct download from tape is prohibited. '
diff --git a/lib/rucio/tests/test_download.py b/lib/rucio/tests/test_download.py
index cce177504..b894d3f6f 100644
--- a/lib/rucio/tests/test_download.py
+++ b/lib/rucio/tests/test_download.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Copyright 2019-2021 CERN
+# Copyright 2019-2022 CERN
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -24,7 +24,7 @@
 # - Radu Carpa <radu.carpa@cern.ch>, 2021
 # - Martin Barisits <martin.barisits@cern.ch>, 2021
 # - Rakshita Varadarajan <rakshitajps@gmail.com>, 2021
-# - Joel Dierkes <joel.dierkes@cern.ch>, 2021
+# - Joel Dierkes <joel.dierkes@cern.ch>, 2021-2022
 
 import logging
 import os
@@ -723,3 +723,16 @@ def test_download_file_with_supported_protocol_from_config(rse_factory, did_fact
             patch('rucio.rse.protocols.%s.Default.close' % supported_impl):
         download_client.download_dids([{'did': did_str, 'impl': supported_impl}])
         mock_get.assert_called()
+
+
+def test_download_exclude_tape(rse_factory, did_factory, download_client):
+    """Client: Do not download from a tape rse."""
+    rse, rse_id = rse_factory.make_posix_rse()
+    did = did_factory.upload_test_file(rse)
+    did_str = '%s:%s' % (did['scope'], did['name'])
+
+    # We can not mock the server core code here, so mock the API
+    with patch('rucio.client.rseclient.RSEClient.list_rses', return_value=[{'rse': rse}]), \\
+         TemporaryDirectory() as tmp_dir, \\
+         pytest.raises(NoFilesDownloaded):
+        download_client.download_dids([{'did': did_str, 'base_dir': tmp_dir}])