Repository: rucio/rucio
Issue: Fix variable initialization in delete_replica

Motivation
----------
The rowcount variable is not properly reinitialized in `delete_replicas`




Changed files:
lib/rucio/api/replica.py, lib/rucio/core/did.py, lib/rucio/transfertool/fts3.py, lib/rucio/daemons/bb8/common.py, lib/rucio/core/replica.py, lib/rucio/core/transfer.py, tools/install_venv.py, lib/rucio/web/rest/webpy/v1/replica.py, lib/rucio/client/replicaclient.py, lib/rucio/tests/test_undertaker.py

diff --git a/lib/rucio/api/replica.py b/lib/rucio/api/replica.py
index eb7c2e372..57dc560f9 100644
--- a/lib/rucio/api/replica.py
+++ b/lib/rucio/api/replica.py
@@ -58,7 +58,8 @@ def list_bad_replicas_status(state=BadFilesStatus.BAD, rse=None, younger_than=No
     if rse is not None:
         rse_id = get_rse_id(rse=rse)
 
-    replicas = replica.list_bad_replicas_status(state=state, rse_id=rse_id, younger_than=younger_than, older_than=older_than, limit=limit, list_pfns=list_pfns)
+    replicas = replica.list_bad_replicas_status(state=state, rse_id=rse_id, younger_than=younger_than,
+                                                older_than=older_than, limit=limit, list_pfns=list_pfns)
     return [api_update_return_dict(r) for r in replicas]
 
 
@@ -251,7 +252,6 @@ def delete_replicas(rse, files, issuer, ignore_availability=False):
 
 
 def update_replicas_states(rse, files, issuer):
-
     """
     Update File replica information and state.
 
@@ -299,7 +299,7 @@ def list_dataset_replicas_vp(scope, name, deep=False):
     :param name: The name of the dataset.
     :param deep: Lookup at the file level.
 
-    :returns: If VP exists a list of dicts of sites, otherwise a list of dicts of dataset replicas
+    :returns: If VP exists a list of dicts of sites, otherwise nothing
 
     NOTICE: This is an RnD function and might change or go away at any time.
     """
diff --git a/lib/rucio/client/replicaclient.py b/lib/rucio/client/replicaclient.py
index 8400321da..184b1a5c9 100644
--- a/lib/rucio/client/replicaclient.py
+++ b/lib/rucio/client/replicaclient.py
@@ -280,7 +280,7 @@ class ReplicaClient(BaseClient):
         :param name: The name of the dataset.
         :param deep: Lookup at the file level.
 
-        :returns: If VP exists a list of dicts of sites, otherwise a list of dicts of dataset replicas
+        :returns: If VP exists a list of dicts of sites
         """
         payload = {}
         if deep:
diff --git a/lib/rucio/core/did.py b/lib/rucio/core/did.py
index 67a1697d3..4be1f3402 100644
--- a/lib/rucio/core/did.py
+++ b/lib/rucio/core/did.py
@@ -571,6 +571,7 @@ def delete_dids(dids, account, expire_rules=False, session=None):
     collection_replica_clause, file_clause = [], []
     not_purge_replicas = []
     did_followed_clause = []
+    metadata_to_delete = []
 
     for did in dids:
         logging.info('Removing did %(scope)s:%(name)s (%(did_type)s)' % did)
@@ -623,6 +624,13 @@ def delete_dids(dids, account, expire_rules=False, session=None):
         parent_content_clause.append(and_(models.DataIdentifierAssociation.child_scope == did['scope'], models.DataIdentifierAssociation.child_name == did['name']))
         rule_id_clause.append(and_(models.ReplicationRule.scope == did['scope'], models.ReplicationRule.name == did['name']))
 
+        if session.bind.dialect.name == 'oracle':
+            oracle_version = int(session.connection().connection.version.split('.')[0])
+            if oracle_version >= 12:
+                metadata_to_delete.append(and_(models.DidMeta.scope == did['scope'], models.DidMeta.name == did['name']))
+        else:
+            metadata_to_delete.append(and_(models.DidMeta.scope == did['scope'], models.DidMeta.name == did['name']))
+
         # Send message
         add_message('ERASE', {'account': account.external,
                               'scope': did['scope'].external,
@@ -679,6 +687,19 @@ def delete_dids(dids, account, expire_rules=False, session=None):
             rowcount = session.query(models.CollectionReplica).filter(or_(*collection_replica_clause)).\\
                 delete(synchronize_session=False)
 
+    # Remove generic did metadata
+    if metadata_to_delete:
+        if session.bind.dialect.name == 'oracle':
+            oracle_version = int(session.connection().connection.version.split('.')[0])
+            if oracle_version >= 12:
+                with record_timer_block('undertaker.did_meta'):
+                    rowcount = session.query(models.DidMeta).filter(or_(*metadata_to_delete)).\\
+                        delete(synchronize_session=False)
+        else:
+            with record_timer_block('undertaker.did_meta'):
+                rowcount = session.query(models.DidMeta).filter(or_(*metadata_to_delete)).\\
+                    delete(synchronize_session=False)
+
     # remove data identifier
     if existing_parent_dids:
         # Exit method early to give Judge time to remove locks (Otherwise, due to foreign keys, did removal does not work
diff --git a/lib/rucio/core/replica.py b/lib/rucio/core/replica.py
index e0bdf8410..1af52f0e9 100644
--- a/lib/rucio/core/replica.py
+++ b/lib/rucio/core/replica.py
@@ -14,7 +14,7 @@
 #
 # Authors:
 # - Vincent Garonne <vgaronne@gmail.com>, 2013-2018
-# - Cedric Serfon <cedric.serfon@cern.ch>, 2013-2019
+# - Cedric Serfon <cedric.serfon@cern.ch>, 2013-2020
 # - Ralph Vigne <ralph.vigne@cern.ch>, 2013-2014
 # - Martin Barisits <martin.barisits@cern.ch>, 2013-2019
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2014-2019
@@ -1407,6 +1407,7 @@ def delete_replicas(rse_id, files, ignore_availability=True, session=None):
             filter(or_(*chunk)).\\
             delete(synchronize_session=False)
 
+    rowcount = 0
     for chunk in chunks(replica_condition, 10):
         for (scope, name, rid, replica_bytes) in session.query(models.RSEFileAssociation.scope, models.RSEFileAssociation.name, models.RSEFileAssociation.rse_id, models.RSEFileAssociation.bytes).\\
                 with_hint(models.RSEFileAssociation, "INDEX(REPLICAS REPLICAS_PK)", 'oracle').filter(models.RSEFileAssociation.rse_id == rse_id).filter(or_(*chunk)):
@@ -2340,7 +2341,7 @@ def list_dataset_replicas_vp(scope, name, deep=False, session=None):
     :param deep: Lookup at the file level.
     :param session: Database session to use.
 
-    :returns: If VP exists a list of dicts ofsites, otherwise a list of dicts of dataset replicas
+    :returns: If VP exists and there is at least one non-TAPE replica, returns a list of dicts of sites
     """
 
     vp_replies = ['other']
@@ -2354,17 +2355,28 @@ def list_dataset_replicas_vp(scope, name, deep=False, session=None):
             vp_replies = vp_replies.json()
         else:
             vp_replies = ['other']
-    except Exception as e:
-        print(e)
+    except Exception as exc:
+        print(exc)
         vp_replies = ['other']
 
     if vp_replies != ['other']:
-        for vp_reply in vp_replies:
-            yield {'vp': True, 'site': vp_reply}
-    else:
+        # check that there is at least one regular replica
+        # that is not on tape and has a protocol  with scheme "root"
+        # and can be accessed from WAN
+        accessible_replica_exists = False
         for reply in list_dataset_replicas(scope=scope, name=name, deep=deep, session=session):
-            reply['vp'] = False
-            yield reply
+            rse_info = rsemgr.get_rse_info(rse=reply['rse'], session=session)
+            if rse_info['rse_type'] == 'TAPE':
+                continue
+            for prot in rse_info['protocols']:
+                if prot['scheme'] == 'root' and prot['domains']['wan']['read']:
+                    accessible_replica_exists = True
+                    break
+            if accessible_replica_exists is True:
+                break
+        if accessible_replica_exists is True:
+            for vp_reply in vp_replies:
+                yield {'vp': True, 'site': vp_reply}
 
 
 @stream_session
diff --git a/lib/rucio/core/transfer.py b/lib/rucio/core/transfer.py
index 17891225b..d14836f6b 100644
--- a/lib/rucio/core/transfer.py
+++ b/lib/rucio/core/transfer.py
@@ -1,18 +1,30 @@
-# Copyright European Organization for Nuclear Research (CERN)
+# Copyright 2013-2020 CERN for the benefit of the ATLAS collaboration.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
-# You may not use this file except in compliance with the License.
+# you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
-# http://www.apache.org/licenses/LICENSE-2.0
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
 #
 # Authors:
-# - Martin Barisits, <martin.barisits@cern.ch>, 2017-2018
-# - Mario Lassnig, <mario.lassnig@cern.ch>, 2017-2018
-# - Cedric Serfon, <cedric.serfon@cern.ch>, 2018-2020
-# - Hannes Hansen, <hannes.jakob.hansen@cern.ch>, 2018
-# - Robert Illingworth, <illingwo@fnal.gov>, 2019
-# - Andrew Lister, <andrew.lister@stfc.ac.uk>, 2019
-# - Brandon White, <bjwhite@fnal.gov>, 2019
+# - Mario Lassnig <mario.lassnig@cern.ch>, 2013-2020
+# - Martin Barisits <martin.barisits@cern.ch>, 2017-2019
+# - Vincent Garonne <vgaronne@gmail.com>, 2017
+# - Igor Mandrichenko <rucio@fermicloud055.fnal.gov>, 2018
+# - Cedric Serfon <cedric.serfon@cern.ch>, 2018-2020
+# - dciangot <diego.ciangottini@cern.ch>, 2018
+# - Robert Illingworth <illingwo@fnal.gov>, 2018-2019
+# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+# - Andrew Lister <andrew.lister@stfc.ac.uk>, 2019
+# - Brandon White <bjwhite@fnal.gov>, 2019
+# - maatthias <maatthias@gmail.com>, 2019
+# - Gabriele <sucre.91@hotmail.it>, 2019
 #
 # PY3K COMPATIBLE
 
@@ -832,7 +844,7 @@ def get_transfer_requests_and_source_replicas(total_workers=0, worker_number=0,
                                      'dest_spacetoken': dest_spacetoken,
                                      'overwrite': overwrite,
                                      'bring_online': bring_online,
-                                     'copy_pin_lifetime': dict_attributes.get('lifetime', -1),
+                                     'copy_pin_lifetime': dict_attributes.get('lifetime', 172800),
                                      'external_host': external_host,
                                      'selection_strategy': 'auto',
                                      'rule_id': rule_id,
diff --git a/lib/rucio/daemons/bb8/common.py b/lib/rucio/daemons/bb8/common.py
index cd2070673..91410cf78 100644
--- a/lib/rucio/daemons/bb8/common.py
+++ b/lib/rucio/daemons/bb8/common.py
@@ -197,6 +197,9 @@ def _list_rebalance_rule_candidates_dump(rse, mode=None):
                     rules[rule_id] = {'state': 'DELETED'}
                     print(e)
                     continue
+                if rule_info['child_rule_id']:
+                    rules[rule_id] = {'state': 'DELETED'}
+                    continue
                 rules[rule_id] = {'scope': rule_info['scope'],
                                   'name': rule_info['name'],
                                   'rse_expression': rse_expression,
diff --git a/lib/rucio/tests/test_undertaker.py b/lib/rucio/tests/test_undertaker.py
index 7e8ec76e0..f1fbc81e6 100644
--- a/lib/rucio/tests/test_undertaker.py
+++ b/lib/rucio/tests/test_undertaker.py
@@ -27,7 +27,7 @@ from nose.tools import assert_not_equal
 from rucio.common.types import InternalAccount, InternalScope
 from rucio.common.utils import generate_uuid
 from rucio.core.account_limit import set_local_account_limit
-from rucio.core.did import add_dids, attach_dids, list_expired_dids, get_did
+from rucio.core.did import add_dids, attach_dids, list_expired_dids, get_did, add_did_meta
 from rucio.core.replica import get_replica
 from rucio.core.rule import add_rules, list_rules
 from rucio.core.rse import get_rse_id, add_rse
@@ -65,6 +65,14 @@ class TestUndertaker:
 
         add_dids(dids=dsns1 + dsns2, account=root)
 
+        # Add generic metadata on did
+        test_metadata = {"test_key": "test_value"}
+        try:
+            add_did_meta(tmp_scope, dsns1[0]['name'], test_metadata)
+        except NotImplementedError:
+            # add_did_meta is not Implemented for Oracle < 12
+            pass
+
         replicas = list()
         for dsn in dsns1 + dsns2:
             files = [{'scope': tmp_scope, 'name': 'file_%s' % generate_uuid(),
diff --git a/lib/rucio/transfertool/fts3.py b/lib/rucio/transfertool/fts3.py
index a1f85515e..3f6c816ba 100644
--- a/lib/rucio/transfertool/fts3.py
+++ b/lib/rucio/transfertool/fts3.py
@@ -700,8 +700,8 @@ class FTS3Transfertool(Transfertool):
                                      'src_url': file_resp.get('source_surl', None),
                                      'dst_url': file_resp.get('dest_surl', None),
                                      'started_at': datetime.datetime.strptime(file_resp['start_time'], '%Y-%m-%dT%H:%M:%S') if file_resp['start_time'] else None,
-                                     'staging_start_at': datetime.datetime.strptime(file_resp['staging_start'], '%Y-%m-%dT%H:%M:%S') if file_resp['staging_start'] else None,
-                                     'staging_finished_at': datetime.datetime.strptime(file_resp['staging_finished'], '%Y-%m-%dT%H:%M:%S') if file_resp['staging_finished'] else None,
+                                     'staging_start': datetime.datetime.strptime(file_resp['staging_start'], '%Y-%m-%dT%H:%M:%S') if file_resp['staging_start'] else None,
+                                     'staging_finished': datetime.datetime.strptime(file_resp['staging_finished'], '%Y-%m-%dT%H:%M:%S') if file_resp['staging_finished'] else None,
                                      'transferred_at': datetime.datetime.strptime(file_resp['finish_time'], '%Y-%m-%dT%H:%M:%S') if file_resp['finish_time'] else None,
                                      'duration': duration,
                                      'reason': file_resp.get('reason', None),
diff --git a/lib/rucio/web/rest/webpy/v1/replica.py b/lib/rucio/web/rest/webpy/v1/replica.py
index 45b94422b..5b2d3ff42 100755
--- a/lib/rucio/web/rest/webpy/v1/replica.py
+++ b/lib/rucio/web/rest/webpy/v1/replica.py
@@ -205,7 +205,8 @@ class Replicas(RucioController):
             raise generate_http_error(400, 'ValueError', 'Cannot decode json parameter list')
 
         try:
-            add_replicas(rse=parameters['rse'], files=parameters['files'], issuer=ctx.env.get('issuer'), ignore_availability=parameters.get('ignore_availability', False))
+            add_replicas(rse=parameters['rse'], files=parameters['files'], issuer=ctx.env.get(
+                'issuer'), ignore_availability=parameters.get('ignore_availability', False))
         except InvalidPath as error:
             raise generate_http_error(400, 'InvalidPath', error.args[0])
         except AccessDenied as error:
@@ -274,7 +275,8 @@ class Replicas(RucioController):
             raise generate_http_error(400, 'ValueError', 'Cannot decode json parameter list')
 
         try:
-            delete_replicas(rse=parameters['rse'], files=parameters['files'], issuer=ctx.env.get('issuer'), ignore_availability=parameters.get('ignore_availability', False))
+            delete_replicas(rse=parameters['rse'], files=parameters['files'], issuer=ctx.env.get(
+                'issuer'), ignore_availability=parameters.get('ignore_availability', False))
         except AccessDenied as error:
             raise generate_http_error(401, 'AccessDenied', error.args[0])
         except RSENotFound as error:
@@ -677,7 +679,8 @@ class BadReplicasStates(RucioController):
                 list_pfns = bool(params['list_pfns'][0])
 
         try:
-            result = list_bad_replicas_status(state=state, rse=rse, younger_than=younger_than, older_than=older_than, limit=limit, list_pfns=list_pfns)
+            result = list_bad_replicas_status(state=state, rse=rse, younger_than=younger_than,
+                                              older_than=older_than, limit=limit, list_pfns=list_pfns)
         except RucioException as error:
             raise generate_http_error(500, error.__class__.__name__, error.args[0])
         except Exception as error:
@@ -782,7 +785,7 @@ class DatasetReplicasVP(RucioController):
             406 Not Acceptable
             500 InternalError
 
-        :returns: If VP exists a list of dicts of sites, otherwise a list of dicts of dataset replicas
+        :returns: If VP exists a list of dicts of sites, otherwise nothing
         """
 
         header('Content-Type', 'application/x-json-stream')
diff --git a/tools/install_venv.py b/tools/install_venv.py
index b40919bac..c28b882e4 100755
--- a/tools/install_venv.py
+++ b/tools/install_venv.py
@@ -19,9 +19,9 @@ import sys
 
 ROOT = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 VENV = os.path.join(ROOT, '.venv')
-PIP_REQUIRES = os.path.join(ROOT, 'tools', 'pip-requires')
-PIP_REQUIRES_CLIENT = os.path.join(ROOT, 'tools', 'pip-requires-client')
-PIP_REQUIRES_TEST = os.path.join(ROOT, 'tools', 'pip-requires-test')
+PIP_REQUIRES = os.path.join(ROOT, 'etc', 'pip-requires')
+PIP_REQUIRES_CLIENT = os.path.join(ROOT, 'etc', 'pip-requires-client')
+PIP_REQUIRES_TEST = os.path.join(ROOT, 'etc', 'pip-requires-test')
 
 
 def die(message, *args):