Repository: rucio/rucio
Issue: Problem with deleting RSE after deleting attribute

Motivation
----------
If you delete the RSE attribute with the same name as the RSE, you can not delete the RSE afterwards. The request will fail with status code 500 and 'Internal server error' message.
```
DELETE /rses/MOCK/attr/MOCK
DELETE /rses/MOCK
```

Modification
------------
I would create a new exception class e.g. RSEAttributeNotFound and throw it [here](https://github.com/rucio/rucio/blob/master/lib/rucio/core/rse.py#L326) if a NoResultFound exception gets raised and then respond with the corresponding error message.
Then there should be a check if the attribute to be deleted has the same name as the RSE to avoid this exception in the RSE deletion.

Changed files:
lib/rucio/common/exception.py, lib/rucio/web/rest/flaskapi/v1/rse.py, lib/rucio/core/rse.py, lib/rucio/tests/test_rse.py, lib/rucio/web/rest/webpy/v1/rse.py

diff --git a/lib/rucio/common/exception.py b/lib/rucio/common/exception.py
index 752308668..946d0b4ec 100644
--- a/lib/rucio/common/exception.py
+++ b/lib/rucio/common/exception.py
@@ -22,6 +22,7 @@
 # - Cedric Serfon <cedric.serfon@cern.ch>, 2013-2018
 # - Wen Guan <wguan.icedew@gmail.com>, 2014-2015
 # - Tobias Wegner <twegner@cern.ch>, 2018
+# - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
 #
 # PY3K COMPATIBLE
 
@@ -891,3 +892,13 @@ class TransferToolWrongAnswer(RucioException):
         super(TransferToolWrongAnswer, self).__init__(args, kwargs)
         self._message = "Wrong answer returned by the transfer tool."
         self.error_code = 84
+
+
+class RSEAttributeNotFound(RucioException):
+    """
+    RSE attribute not found.
+    """
+    def __init__(self, *args, **kwargs):
+        super(RSEAttributeNotFound, self).__init__(*args, **kwargs)
+        self._message = "RSE attribute not found."
+        self.error_code = 85
diff --git a/lib/rucio/core/rse.py b/lib/rucio/core/rse.py
index 872083e2c..5650a9b67 100644
--- a/lib/rucio/core/rse.py
+++ b/lib/rucio/core/rse.py
@@ -24,7 +24,11 @@
 # - Frank Berghaus <frank.berghaus@cern.ch>, 2018
 # - Dimitrios Christidis <dimitrios.christidis@cern.ch>, 2018
 # - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+#
+# PY3K COMPATIBLE
 
+from __future__ import division
+from builtins import round
 from re import match
 try:
     from StringIO import StringIO
@@ -165,7 +169,10 @@ def del_rse(rse, session=None):
     except sqlalchemy.orm.exc.NoResultFound:
         raise exception.RSENotFound('RSE \\'%s\\' cannot be found' % rse)
     old_rse.delete(session=session)
-    del_rse_attribute(rse=rse, key=rse, session=session)
+    try:
+        del_rse_attribute(rse=rse, key=rse, session=session)
+    except exception.RSEAttributeNotFound:
+        pass
 
 
 @read_session
@@ -275,7 +282,7 @@ def list_rses(filters={}, session=None):
                 query = query.filter(t.value == v)
 
         condition1, condition2 = [], []
-        for i in xrange(0, 8):
+        for i in range(0, 8):
             if i | availability_mask1 == i:
                 condition1.append(models.RSE.availability == i)
             if i & availability_mask2 == i:
@@ -335,8 +342,12 @@ def del_rse_attribute(rse, key, session=None):
     :return: True if RSE attribute was deleted.
     """
     rse_id = get_rse_id(rse, session=session)
-    query = session.query(models.RSEAttrAssociation).filter_by(rse_id=rse_id).filter(models.RSEAttrAssociation.key == key)
-    rse_attr = query.one()
+    rse_attr = None
+    try:
+        query = session.query(models.RSEAttrAssociation).filter_by(rse_id=rse_id).filter(models.RSEAttrAssociation.key == key)
+        rse_attr = query.one()
+    except sqlalchemy.orm.exc.NoResultFound:
+        raise exception.RSEAttributeNotFound('RSE attribute \\'%s\\' cannot be found' % key)
     rse_attr.delete(session=session)
     return True
 
diff --git a/lib/rucio/tests/test_rse.py b/lib/rucio/tests/test_rse.py
index f160f6c96..0df11520f 100644
--- a/lib/rucio/tests/test_rse.py
+++ b/lib/rucio/tests/test_rse.py
@@ -23,6 +23,8 @@
 # - Joaquin Bogado <jbogado@linti.unlp.edu.ar>, 2018
 # - Frank Berghaus <frank.berghaus@cern.ch>, 2018
 # - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+#
+# PY3K COMPATIBLE
 
 from __future__ import print_function
 
@@ -35,11 +37,11 @@ from rucio.client.rseclient import RSEClient
 from rucio.client.replicaclient import ReplicaClient
 from rucio.common.exception import (Duplicate, RSENotFound, RSEProtocolNotSupported,
                                     InvalidObject, RSEProtocolDomainNotSupported, RSEProtocolPriorityError,
-                                    ResourceTemporaryUnavailable)
+                                    ResourceTemporaryUnavailable, RSEAttributeNotFound)
 from rucio.common.utils import generate_uuid
 from rucio.core.rse import (add_rse, get_rse_id, del_rse, list_rses, rse_exists, add_rse_attribute, list_rse_attributes,
-                            set_rse_transfer_limits, get_rse_transfer_limits, delete_rse_transfer_limits, get_rse,
-                            get_rse_protocols)
+                            set_rse_transfer_limits, get_rse_transfer_limits, delete_rse_transfer_limits,
+                            get_rse_protocols, del_rse_attribute, get_rse_attribute, get_rse)
 from rucio.rse import rsemanager as mgr
 from rucio.tests.common import rse_name_generator
 from rucio.web.rest.rse import APP as rse_app
@@ -113,8 +115,8 @@ class TestRSECoreApi(object):
         rse_id = add_rse(rse)
         add_rse_attribute(rse=rse, key='tier', value='1')
         attr = list_rse_attributes(rse=None, rse_id=rse_id)
-        assert_in('tier', attr.keys())
-        assert_in(rse, attr.keys())
+        assert_in('tier', list(attr.keys()))
+        assert_in(rse, list(attr.keys()))
 
     def test_create_and_check_rse_transfer_limits(self):
         """ RSE (CORE): Test the creation, query, and deletion of a RSE transfer limit"""
@@ -128,7 +130,7 @@ class TestRSECoreApi(object):
 
         set_rse_transfer_limits(rse=rse, activity=activity, max_transfers=max_transfers, transfers=transfers, waitings=waitings)
         limits = get_rse_transfer_limits(rse=rse, activity=activity)
-        assert_in(activity, limits.keys())
+        assert_in(activity, list(limits.keys()))
         assert_in(rse_id, limits[activity])
         assert_equal(max_transfers, limits[activity][rse_id]['max_transfers'])
         assert_equal(transfers, limits[activity][rse_id]['transfers'])
@@ -136,7 +138,7 @@ class TestRSECoreApi(object):
 
         set_rse_transfer_limits(rse=rse, activity=activity, max_transfers=max_transfers + 1, transfers=transfers + 1, waitings=waitings + 1)
         limits = get_rse_transfer_limits(rse=rse, activity=activity)
-        assert_in(activity, limits.keys())
+        assert_in(activity, list(limits.keys()))
         assert_in(rse_id, limits[activity])
         assert_equal(max_transfers + 1, limits[activity][rse_id]['max_transfers'])
         assert_equal(transfers + 1, limits[activity][rse_id]['transfers'])
@@ -149,6 +151,16 @@ class TestRSECoreApi(object):
 
         del_rse(rse)
 
+    def test_delete_rse_attribute(self):
+        """ RSE (CORE): Test the deletion of a RSE attribute. """
+        rse_name = rse_name_generator()
+        add_rse(rse_name)
+        del_rse_attribute(rse=rse_name, key=rse_name)
+        assert_equal(get_rse_attribute(key=rse_name, rse_id=get_rse_id(rse_name)), [])
+
+        with assert_raises(RSEAttributeNotFound):
+            del_rse_attribute(rse=rse_name, key=rse_name)
+
 
 class TestRSE(object):
 
@@ -258,6 +270,22 @@ class TestRSE(object):
         r2 = TestApp(rse_app.wsgifunc(*mw)).get('/MOCK/accounts/usage', headers=headers2, expect_errors=True)
         assert_equal(r2.status, 200)
 
+    def test_delete_rse_attribute(self):
+        """ RSE (REST): Test the deletion of a RSE attribute """
+        mw = []
+        headers1 = {'X-Rucio-Account': 'root', 'X-Rucio-Username': 'ddmlab', 'X-Rucio-Password': 'secret'}
+        r1 = TestApp(auth_app.wsgifunc(*mw)).get('/userpass', headers=headers1, expect_errors=True)
+        token = str(r1.header('X-Rucio-Auth-Token'))
+
+        rse_name = rse_name_generator()
+        add_rse(rse_name)
+        headers2 = {'X-Rucio-Type': 'user', 'X-Rucio-Account': 'root', 'X-Rucio-Auth-Token': str(token)}
+        r2 = TestApp(rse_app.wsgifunc(*mw)).delete('/{0}/attr/{0}'.format(rse_name), headers=headers2, expect_errors=True)
+        assert_equal(r2.status, 200)
+
+        r2 = TestApp(rse_app.wsgifunc(*mw)).delete('/{0}/attr/{0}'.format(rse_name), headers=headers2, expect_errors=True)
+        assert_equal(r2.status, 404)
+
 
 class TestRSEClient(object):
 
@@ -1390,3 +1418,13 @@ class TestRSEClient(object):
         assert_equal(info['verify_checksum'], True)
 
         del_rse(rse)
+
+    def test_delete_rse_attribute(self):
+        """ RSE (CLIENT): Test the deletion of a RSE attribute. """
+        rse_name = rse_name_generator()
+        self.client.add_rse(rse_name)
+        self.client.delete_rse_attribute(rse=rse_name, key=rse_name)
+        assert_equal(get_rse_attribute(key=rse_name, rse_id=get_rse_id(rse_name)), [])
+
+        with assert_raises(RSEAttributeNotFound):
+            self.client.delete_rse_attribute(rse=rse_name, key=rse_name)
diff --git a/lib/rucio/web/rest/flaskapi/v1/rse.py b/lib/rucio/web/rest/flaskapi/v1/rse.py
index 93509c505..6c12e94aa 100755
--- a/lib/rucio/web/rest/flaskapi/v1/rse.py
+++ b/lib/rucio/web/rest/flaskapi/v1/rse.py
@@ -21,6 +21,8 @@
 # - Martin Barisits <martin.barisits@cern.ch>, 2017
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
 # - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+#
+# PY3K COMPATIBLE
 
 from __future__ import print_function
 from json import dumps, loads
@@ -39,7 +41,8 @@ from rucio.api.rse import (add_rse, update_rse, list_rses, del_rse, add_rse_attr
 from rucio.common.exception import (Duplicate, AccessDenied, RSENotFound, RucioException,
                                     RSEOperationNotSupported, RSEProtocolNotSupported,
                                     InvalidObject, RSEProtocolDomainNotSupported,
-                                    RSEProtocolPriorityError, InvalidRSEExpression)
+                                    RSEProtocolPriorityError, InvalidRSEExpression,
+                                    RSEAttributeNotFound)
 from rucio.common.utils import generate_http_error_flask, render_json, APIEncoder
 from rucio.web.rest.flaskapi.v1.common import before_request, after_request
 from rucio.rse import rsemanager
@@ -271,7 +274,7 @@ class Attributes(MethodView):
         except AccessDenied as error:
             return generate_http_error_flask(401, 'AccessDenied', error.args[0])
         except Duplicate as error:
-            return generate_http_error_flask(409, 'Duplicate', error[0])
+            return generate_http_error_flask(409, 'Duplicate', error.args[0])
         except Exception as error:
             return error, 500
 
@@ -310,6 +313,7 @@ class Attributes(MethodView):
         :status 200: OK.
         :status 401: Invalid Auth Token.
         :status 404: RSE not found.
+        :status 404: RSE attribute not found.
         :status 500: Internal Error.
 
         """
@@ -319,6 +323,8 @@ class Attributes(MethodView):
             return generate_http_error_flask(401, 'AccessDenied', error.args[0])
         except RSENotFound as error:
             return generate_http_error_flask(404, 'RSENotFound', error.args[0])
+        except RSEAttributeNotFound as error:
+            return generate_http_error_flask(404, 'RSEAttributeNotFound', error.args[0])
         except Exception as error:
             return error, 500
 
diff --git a/lib/rucio/web/rest/webpy/v1/rse.py b/lib/rucio/web/rest/webpy/v1/rse.py
index e3b3e11fb..ab3bca445 100755
--- a/lib/rucio/web/rest/webpy/v1/rse.py
+++ b/lib/rucio/web/rest/webpy/v1/rse.py
@@ -21,11 +21,16 @@
 # - Martin Barisits <martin.barisits@cern.ch>, 2017
 # - Mario Lassnig <mario.lassnig@cern.ch>, 2018
 # - Hannes Hansen <hannes.jakob.hansen@cern.ch>, 2018
+#
+# PY3K COMPATIBLE
 
 from __future__ import print_function
 from json import dumps, loads
 from traceback import format_exc
-from urlparse import parse_qs, parse_qsl
+try:
+    from urlparse import parse_qs, parse_qsl
+except ImportError:
+    from urllib.parse import parse_qs, parse_qsl
 from web import (application, ctx, data, header, Created, InternalError, OK,
                  input, loadhook)
 
@@ -40,7 +45,8 @@ from rucio.api.rse import (add_rse, update_rse, list_rses, del_rse, add_rse_attr
 from rucio.common.exception import (Duplicate, AccessDenied, RSENotFound, RucioException,
                                     RSEOperationNotSupported, RSEProtocolNotSupported,
                                     InvalidObject, RSEProtocolDomainNotSupported,
-                                    RSEProtocolPriorityError, InvalidRSEExpression)
+                                    RSEProtocolPriorityError, InvalidRSEExpression,
+                                    RSEAttributeNotFound)
 from rucio.common.utils import generate_http_error, render_json, APIEncoder
 from rucio.web.rest.common import rucio_loadhook, RucioController
 from rucio.rse import rsemanager
@@ -211,7 +217,7 @@ class RSE(RucioController):
             rse_prop = get_rse(rse=rse)
             return render_json(**rse_prop)
         except RSENotFound as error:
-            raise generate_http_error(404, 'RSENotFound', error[0])
+            raise generate_http_error(404, 'RSENotFound', error.args[0])
         except RucioException as error:
             raise generate_http_error(500, error.__class__.__name__, error.args[0])
 
@@ -307,12 +313,22 @@ class Attributes(RucioController):
         return dumps(rse_attr)
 
     def DELETE(self, rse, key):
+        """ delete RSE attribute
+         HTTP Success:
+            200 OK
+         HTTP Error:
+            401 Unauthorized
+            404 Not Found
+            500 InternalError
+        """
         try:
             del_rse_attribute(rse=rse, key=key, issuer=ctx.env.get('issuer'))
         except AccessDenied as error:
             raise generate_http_error(401, 'AccessDenied', error.args[0])
         except RSENotFound as error:
             raise generate_http_error(404, 'RSENotFound', error.args[0])
+        except RSEAttributeNotFound as error:
+            raise generate_http_error(404, 'RSEAttributeNotFound', error.args[0])
         except Exception as error:
             raise InternalError(error)