SEBA-447 Check for closed db connections;
Remove dependency on django-rest-framework's exceptions
Change-Id: I440c459f1a65321e25d4f7794a24a9a4d92dbb00
diff --git a/lib/xos-genx/xosgenx/targets/grpc_api.xtarget b/lib/xos-genx/xosgenx/targets/grpc_api.xtarget
index 387a2fe..2d8a032 100644
--- a/lib/xos-genx/xosgenx/targets/grpc_api.xtarget
+++ b/lib/xos-genx/xosgenx/targets/grpc_api.xtarget
@@ -7,7 +7,7 @@
from django.contrib.auth import authenticate as django_authenticate
from xos.exceptions import *
from apihelper import XOSAPIHelperMixin
-from decorators import translate_exceptions
+from decorators import translate_exceptions, check_db_connection
import grpc
class XosService(xos_pb2_grpc.xosServicer, XOSAPIHelperMixin):
@@ -22,6 +22,7 @@
{%- if object.name!='XOSBase' %}
@translate_exceptions("{{ object.name }}", "List{{ object.name }}")
@track_request_time("{{ object.name }}", "List{{ object.name }}")
+ @check_db_connection
def List{{ object.name }}(self, request, context):
user=self.authenticate(context)
model=self.get_model("{{ object.name }}")
@@ -31,6 +32,7 @@
@translate_exceptions("{{ object.name }}", "Filter{{ object.name }}")
@track_request_time("{{ object.name }}", "Filter{{ object.name }}")
+ @check_db_connection
def Filter{{ object.name }}(self, request, context):
user=self.authenticate(context)
model=self.get_model("{{ object.name }}")
@@ -40,6 +42,7 @@
@translate_exceptions("{{ object.name }}", "Get{{ object.name }}")
@track_request_time("{{ object.name }}", "Get{{ object.name }}")
+ @check_db_connection
def Get{{ object.name }}(self, request, context):
user=self.authenticate(context)
model=self.get_model("{{ object.name }}")
@@ -49,6 +52,7 @@
@translate_exceptions("{{ object.name }}", "Create{{ object.name }}")
@track_request_time("{{ object.name }}", "Create{{ object.name }}")
+ @check_db_connection
def Create{{ object.name }}(self, request, context):
user=self.authenticate(context)
model=self.get_model("{{ object.name }}")
@@ -58,6 +62,7 @@
@translate_exceptions("{{ object.name }}", "Delete{{ object.name }}")
@track_request_time("{{ object.name }}", "Delete{{ object.name }}")
+ @check_db_connection
def Delete{{ object.name }}(self, request, context):
user=self.authenticate(context)
model=self.get_model("{{ object.name }}")
@@ -67,6 +72,7 @@
@translate_exceptions("{{ object.name }}", "Update{{ object.name }}")
@track_request_time("{{ object.name }}", "Update{{ object.name }}")
+ @check_db_connection
def Update{{ object.name }}(self, request, context):
user=self.authenticate(context)
model=self.get_model("{{ object.name }}")
diff --git a/tox.ini b/tox.ini
index a8ce26b..084a487 100644
--- a/tox.ini
+++ b/tox.ini
@@ -29,6 +29,7 @@
nose2
flake8
pyfakefs
+ grpcio==1.19.0
commands =
nose2 -c tox.ini --verbose --junit-xml
diff --git a/xos/coreapi/backupsetwatcher.py b/xos/coreapi/backupsetwatcher.py
index 6d2fe1c..e81194e 100644
--- a/xos/coreapi/backupsetwatcher.py
+++ b/xos/coreapi/backupsetwatcher.py
@@ -39,6 +39,7 @@
from core.models import BackupFile, BackupOperation
from django.db.models import Q, F
+from decorators import check_db_connection
from xosconfig import Config
from multistructlog import create_logger
@@ -234,6 +235,7 @@
with open(request_fn, "w") as f:
f.write(json.dumps(request))
+ @check_db_connection
def run_once(self):
# If we're exiting due to a backup request being saved, then return
if self.exiting:
diff --git a/xos/coreapi/decorators.py b/xos/coreapi/decorators.py
index bd543ca..dc3d6d7 100644
--- a/xos/coreapi/decorators.py
+++ b/xos/coreapi/decorators.py
@@ -16,6 +16,7 @@
from apistats import REQUEST_COUNT
import time
import grpc
+from django import db
from xos.exceptions import (
XOSNotAuthenticated,
@@ -29,6 +30,9 @@
log = create_logger(Config().get("logging"))
+# This decorator causes issues with unit tests, so provide a means to disable it.
+disable_check_db_connection_decorator = False
+
def translate_exceptions(model, method):
""" this decorator translates XOS exceptions to grpc status codes """
@@ -111,3 +115,31 @@
return result
return wrapper
+
+
+def check_db_connection(function):
+ """ Check that the database connection is not in "already closed" state.
+ This tends to happen when postgres is restarted. Connections will persist
+ in this state throwing "connection already closed" errors until the
+ old connections are disposed of.
+ """
+ def wrapper(self, *args, **kwargs):
+ if not disable_check_db_connection_decorator:
+ try:
+ db.connection.cursor()
+ except Exception as e:
+ if "connection already closed" in str(e):
+ log.warning("check_db_connection: connection already closed")
+ try:
+ db.close_old_connections()
+ log.info("check_db_connection: removed old connections")
+ except Exception as e:
+ log.exception("check_db_connection: we failed to fix the failure", e=e)
+ raise
+ else:
+ raise
+
+ result = function(self, *args, **kwargs)
+ return result
+
+ return wrapper
diff --git a/xos/coreapi/reaper.py b/xos/coreapi/reaper.py
index 6080e1a..9928656 100644
--- a/xos/coreapi/reaper.py
+++ b/xos/coreapi/reaper.py
@@ -35,6 +35,7 @@
sys.path.append("/opt/xos")
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "xos.settings")
+from decorators import check_db_connection
from xosconfig import Config
from multistructlog import create_logger
log = create_logger(Config().get("logging"))
@@ -50,24 +51,6 @@
self.terminate_signal = False
super(ReaperThread, self).__init__(*args, **kwargs)
- def check_db_connection_okay(self):
- # django implodes if the database connection is closed by docker-compose
- from django import db
-
- try:
- db.connection.cursor()
- except Exception as e:
- if "connection already closed" in traceback.format_exc():
- log.exception("XXX connection already closed", e=e)
- try:
- # if db.connection:
- # db.connection.close()
- db.close_old_connections()
- except Exception as e:
- log.exception("XXX we failed to fix the failure", e=e)
- else:
- log.exception("XXX some other error", e=e)
-
def journal_object(self, o, operation, msg=None, timestamp=None):
# not implemented at this time
pass
@@ -95,11 +78,10 @@
deps.append(model)
return deps
+ @check_db_connection
def run_reaper_once(self):
# logger.debug("REAPER: run_reaper_once()")
- self.check_db_connection_okay()
-
# Reap non-sync'd models here
# models_to_reap = [Slice,Network,NetworkSlice]
diff --git a/xos/coreapi/test_backupsetwatcher.py b/xos/coreapi/test_backupsetwatcher.py
index 3a17441..7793f23 100644
--- a/xos/coreapi/test_backupsetwatcher.py
+++ b/xos/coreapi/test_backupsetwatcher.py
@@ -20,9 +20,9 @@
from mock import MagicMock, Mock, patch
from pyfakefs import fake_filesystem_unittest
from io import open
-
from xosconfig import Config
+XOS_DIR = os.path.join(os.path.dirname(__file__), "..")
def make_model(vars_dict, var_name, **kwargs):
""" Helper function to mock creating objects. Creates a MagicMock with the
@@ -61,6 +61,13 @@
sys.modules["core.models"] = Mock(BackupOperation=self.mock_backup_operation,
BackupFile=self.mock_backup_file)
+ # needed because decorators.py imports xos.exceptions
+ self.sys_path_save = sys.path
+ sys.path = [XOS_DIR] + sys.path
+
+ import decorators
+ decorators.disable_check_db_connection_decorator = True
+
import backupsetwatcher
self.backupsetwatcher = backupsetwatcher
@@ -70,7 +77,7 @@
self.watcher = backupsetwatcher.BackupSetWatcherThread(self.server)
def tearDown(self):
- pass
+ sys.path = self.sys_path_save
def test_init_request_dir(self):
self.assertFalse(os.path.exists(self.watcher.backup_request_dir))
diff --git a/xos/coreapi/xos_dynamicload_api.py b/xos/coreapi/xos_dynamicload_api.py
index 1ee3bcb..c58daba 100644
--- a/xos/coreapi/xos_dynamicload_api.py
+++ b/xos/coreapi/xos_dynamicload_api.py
@@ -18,7 +18,7 @@
from dynamicbuild import DynamicBuilder
from apistats import REQUEST_COUNT, track_request_time
from authhelper import XOSAuthHelperMixin
-from decorators import translate_exceptions, require_authentication
+from decorators import translate_exceptions, require_authentication, check_db_connection
import grpc
import semver
from xosconfig import Config
@@ -71,6 +71,7 @@
@track_request_time("DynamicLoad", "LoadModels")
@translate_exceptions("DynamicLoad", "LoadModels")
+ @check_db_connection
@require_authentication
def LoadModels(self, request, context):
try:
@@ -176,6 +177,7 @@
@track_request_time("DynamicLoad", "UnloadModels")
@translate_exceptions("DynamicLoad", "UnloadModels")
+ @check_db_connection
@require_authentication
def UnloadModels(self, request, context):
try:
@@ -205,6 +207,7 @@
@track_request_time("DynamicLoad", "GetLoadStatus")
@translate_exceptions("DynamicLoad", "GetLoadStatus")
+ @check_db_connection
@require_authentication
def GetLoadStatus(self, request, context):
try:
@@ -249,6 +252,7 @@
@track_request_time("DynamicLoad", "GetConvenienceMethods")
@translate_exceptions("DynamicLoad", "GetConvenienceMethods")
+ @check_db_connection
@require_authentication
def GetConvenienceMethods(self, request, context):
# self.authenticate(context, required=True)
diff --git a/xos/coreapi/xos_filetransfer_api.py b/xos/coreapi/xos_filetransfer_api.py
index 4109540..1eb01d4 100644
--- a/xos/coreapi/xos_filetransfer_api.py
+++ b/xos/coreapi/xos_filetransfer_api.py
@@ -14,7 +14,7 @@
from apistats import track_request_time
from authhelper import XOSAuthHelperMixin
-from decorators import translate_exceptions, require_authentication
+from decorators import translate_exceptions, require_authentication, check_db_connection
import os
from protos import filetransfer_pb2, filetransfer_pb2_grpc
@@ -35,6 +35,7 @@
@translate_exceptions("FileTransfer", "Download")
@track_request_time("FileTransfer", "Download")
+ @check_db_connection
@require_authentication
def Download(self, request, context):
if not request.uri.startswith("file://"):
@@ -53,6 +54,7 @@
@translate_exceptions("FileTransfer", "Upload")
@track_request_time("FileTransfer", "Upload")
+ @check_db_connection
@require_authentication
def Upload(self, request_iterator, context):
backend_file = None
diff --git a/xos/coreapi/xos_utility_api.py b/xos/coreapi/xos_utility_api.py
index 46514a7..3ebe405 100644
--- a/xos/coreapi/xos_utility_api.py
+++ b/xos/coreapi/xos_utility_api.py
@@ -17,7 +17,7 @@
from apistats import REQUEST_COUNT, track_request_time
import grpc
from authhelper import XOSAuthHelperMixin
-from decorators import translate_exceptions, require_authentication
+from decorators import translate_exceptions, require_authentication, check_db_connection
from xos.exceptions import XOSNotAuthenticated
from core.models import ServiceInstance
from django.db.models import F, Q
@@ -79,6 +79,7 @@
@translate_exceptions("Utilities", "Login")
@track_request_time("Utilities", "Login")
+ @check_db_connection
def Login(self, request, context):
if not request.username:
raise XOSNotAuthenticated("No username")
@@ -104,6 +105,7 @@
@translate_exceptions("Utilities", "Logout")
@track_request_time("Utilities", "Logout")
+ @check_db_connection
def Logout(self, request, context):
for (k, v) in context.invocation_metadata():
if k.lower() == "x-xossession":
@@ -122,6 +124,7 @@
@translate_exceptions("Utilities", "AuthenticatedNoOp")
@track_request_time("Utilities", "AuthenticatedNoOp")
+ @check_db_connection
@require_authentication
def AuthenticatedNoOp(self, request, context):
REQUEST_COUNT.labels(
@@ -131,6 +134,7 @@
@translate_exceptions("Utilities", "ListDirtyModels")
@track_request_time("Utilities", "ListDirtyModels")
+ @check_db_connection
@require_authentication
def ListDirtyModels(self, request, context):
dirty_models = utility_pb2.ModelList()
@@ -159,6 +163,7 @@
@translate_exceptions("Utilities", "SetDirtyModels")
@track_request_time("Utilities", "SetDirtyModels")
+ @check_db_connection
@require_authentication
def SetDirtyModels(self, request, context):
user = self.authenticate(context, required=True)
@@ -196,6 +201,7 @@
return dirty_models
@translate_exceptions("Utilities", "GetXproto")
+ @check_db_connection
@track_request_time("Utilities", "GetXproto")
# TODO(smbaker): Tosca engine calls this without authentication
def GetXproto(self, request, context):
@@ -233,6 +239,7 @@
@translate_exceptions("Utilities", "GetPopulatedServiceInstances")
@track_request_time("Utilities", "GetPopulatedServiceInstances")
+ @check_db_connection
@require_authentication
def GetPopulatedServiceInstances(self, request, context):
"""
diff --git a/xos/xos/exceptions.py b/xos/xos/exceptions.py
index 1033138..01de305 100644
--- a/xos/xos/exceptions.py
+++ b/xos/xos/exceptions.py
@@ -14,8 +14,6 @@
import json
-from rest_framework.exceptions import APIException
-from rest_framework.exceptions import PermissionDenied as RestFrameworkPermissionDenied
def _get_json_error_details(data):
@@ -30,6 +28,30 @@
return json.dumps(ret)
+class APIException(Exception):
+ """
+ Base class for API Exceptions.
+ Subclasses should provide `.status_code` and `.default_detail` properties.
+ """
+ status_code = 500
+ default_detail = {"error": "APIException",
+ "specific_error": "nonspecific",
+ "fields": {}}
+ default_code = 'error'
+
+ def __init__(self, detail=None, code=None):
+ if detail is None:
+ detail = self.default_detail
+ if code is None:
+ code = self.default_code
+
+ self.detail = detail
+ self.code = code # TODO(smbaker): not sure that self.code is useful
+
+ def __str__(self):
+ return str(self.detail)
+
+
class XOSProgrammingError(APIException):
status_code = 400
@@ -44,7 +66,8 @@
self.json_detail = _get_json_error_details(raw_detail)
-class XOSPermissionDenied(RestFrameworkPermissionDenied):
+class XOSPermissionDenied(APIException):
+ status_code = 403
def __init__(self, why="permission error", fields={}):
raw_detail = {
"error": "XOSPermissionDenied",
@@ -56,7 +79,7 @@
self.json_detail = _get_json_error_details(raw_detail)
-class XOSNotAuthenticated(RestFrameworkPermissionDenied):
+class XOSNotAuthenticated(APIException):
status_code = 401
def __init__(self, why="you must be authenticated to use this api", fields={}):
@@ -70,7 +93,7 @@
self.json_detail = _get_json_error_details(raw_detail)
-class XOSNotFound(RestFrameworkPermissionDenied):
+class XOSNotFound(APIException):
status_code = 404
def __init__(self, why="object not found", fields={}):