Skip to content
This repository has been archived by the owner on Jan 3, 2019. It is now read-only.

Creating a restart function for storage classes to fix broken database connections #196

Merged
merged 3 commits into from
Feb 18, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[run]
source = cnxauthoring
branch = True

[report]
# Regexes for lines to exclude from consideration
exclude_lines =
# Have to re-enable the standard pragma
pragma: no cover
raise NotImplementedError
omit =
cnxauthoring/scripts/*
cnxauthoring/tests/*
ignore_errors = True

[html]
directory = coverage_html_report
4 changes: 4 additions & 0 deletions cnxauthoring/storage/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ def search(self, **kwargs):
"""Retrieve any ``Document`` objects from storage that matches the
search terms."""
raise NotImplementedError()

def restart(self):
"""Restart the storage interface """
raise NotImplementedError()
5 changes: 4 additions & 1 deletion cnxauthoring/storage/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ def persist(self):
def abort(self):
"""Persist/commit the changes."""
pass


def restart(self):
"""Restart the interface"""
pass

def search(self, limits, type_=Document, submitter_id=None):
"""Retrieve any ``Document`` objects from storage that matches the
Expand Down
10 changes: 9 additions & 1 deletion cnxauthoring/storage/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@ class PostgresqlStorage(BaseStorage):
Error = psycopg2.Error

def __init__(self, db_connection_string=None):
#initialize db
# initialize db
self.conn = psycopg2.connect(db_connection_string)
# adding a variable to store the db_connection string
# needed to restart the database when a connection
# is broken or lost.
self.db_connection_string = db_connection_string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a variable to store the db_connection string needed to restart the database when a connection is broken or lost.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come to think about it, shouldn't your note be comment in the code?


def get(self, type_=Document, **kwargs):
"""Retrieve ``Document`` objects from storage."""
Expand Down Expand Up @@ -366,3 +370,7 @@ def search(self, limits, type_=Document, submitter_id=None):
for item in res:
yield self._reassemble_model_from_document_entry(**item)
raise StopIteration

def restart(self):
"""Restart the interface"""
self.conn = psycopg2.connect(self.db_connection_string)
60 changes: 60 additions & 0 deletions cnxauthoring/tests/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -2773,6 +2773,33 @@ def test_user_contents_hide_documents_inside_binders(self):
},
})

def test_db_restart(self):
'''
Test to see if the database resets itself after a broken
connection
'''
import psycopg2
from ..storage import storage

self.addCleanup(setattr, storage, 'conn',
psycopg2.connect(storage.conn.dsn))

storage.conn.close()

response = self.testapp.post_json(
'/users/contents',
{'title': u'My document タイトル'},
status=503,
expect_errors=True)
self.assertEqual(response.status, '503 Service Unavailable')

response = self.testapp.post_json(
'/users/contents',
{'title': u'My document タイトル'},
status=201,
expect_errors=True)
self.assertEqual(response.status, '201 Created')

def test_service_unavailable_response(self):
'''
Test service unavailable response when a request is made during a
Expand All @@ -2793,31 +2820,64 @@ def test_service_unavailable_response(self):
expect_errors=True)
self.assertEqual(response.status, '503 Service Unavailable')

storage.conn.close()

response = self.testapp.get(
'/resources/1234abcde',
status=503,
expect_errors=True)
self.assertEqual(response.status, '503 Service Unavailable')

storage.conn.close()

response = self.testapp.put_json(
'/contents/[email protected]',
{},
status=503,
expect_errors=True)
self.assertEqual(response.status, '503 Service Unavailable')

storage.conn.close()

response = self.testapp.get(
'/search',
status=503,
expect_errors=True)
self.assertEqual(response.status, '503 Service Unavailable')

storage.conn.close()

response = self.testapp.delete(
'/contents/{}@draft'.format(id),
status=503,
expect_errors=True)
self.assertEqual(response.status, '503 Service Unavailable')

@mock.patch('cnxauthoring.views.logger')
def test_database_restart_failed(self, logger):
import psycopg2
from ..storage import storage

self.addCleanup(setattr, storage, 'conn',
psycopg2.connect(storage.conn.dsn))

storage.conn.close()

with mock.patch.object(storage, 'restart') as mock_restart:
mock_restart.side_effect = storage.Error

response = self.testapp.post_json(
'/users/contents',
{'title': 'Test Document'},
status=503)
self.assertEqual(mock_restart.call_count, 1)

self.assertEqual(logger.exception.call_count, 3)
args1, args2, args3 = logger.exception.call_args_list
self.assertEqual(args1[0], ('Storage failure',))
self.assertEqual(args2[0], ('Storage failed to abort',))
self.assertEqual(args3[0], ('Storage failed to restart',))


class PublicationTests(BaseFunctionalTestCase):

Expand Down
24 changes: 24 additions & 0 deletions cnxauthoring/tests/test_storage/test_postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,27 @@ def test_add_get_and_remove_binder(self):
self.assertEqual({k: tuple(sorted(v)) for k, v in result.acls.items()},
{'user2': ('view',)})

def test_restart(self):
"""
Testing PostrgressStorage class restart function
with a password connection string.
"""
from ...storage.database import CONNECTION_SETTINGS_KEY
import psycopg2
settings = integration_test_settings()
test_db = settings[CONNECTION_SETTINGS_KEY]
self.addCleanup(setattr, self.storage,
'conn', psycopg2.connect(test_db))

# 0 if the connection is open, nonzero if it is closed or broken.
OPEN = 0

self.assertEqual(self.storage.conn.closed, OPEN)

self.storage.conn.close()

self.assertNotEqual(self.storage.conn.closed, OPEN)

self.storage.restart()

self.assertEqual(self.storage.conn.closed, OPEN)
7 changes: 6 additions & 1 deletion cnxauthoring/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ def wrapper(*args, **kwargs):
storage.abort()
except storage.Error:
logger.exception('Storage failed to abort')
raise httpexceptions.HTTPServiceUnavailable()
try:
storage.restart()
except storage.Error:
logger.exception('Storage failed to restart')
finally:
raise httpexceptions.HTTPServiceUnavailable()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious if there is anyway to make this section more concise without having to use these nested try / except statements

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know a better way to do this.

return wrapper


Expand Down