Don't create fake LogBook when we can not fetch one

When there is no LogBook nor backend in ZookeeperJob or something is
wrong with book data, we now return None from `book` property instead
of new (fake) LogBook. Having two (or more) logbooks with same uuid but
different data is too confusing and makes root cause of the problem
(backend or jobboard misconfiguration) less obvious.

This change also adds `book_name` and `book_uuid` properties to Job
abstract class to provide a way to access book data even if book
itself cannot be fetched.

Change-Id: Iae1f918e35d41794fc348860e6aa3b52ae2211f4
This commit is contained in:
Ivan A. Melnikov
2014-05-15 22:18:45 +04:00
parent 25873576c6
commit 268d935a0a
3 changed files with 51 additions and 21 deletions

View File

@@ -32,7 +32,6 @@ from taskflow.jobs import jobboard
from taskflow.openstack.common import excutils from taskflow.openstack.common import excutils
from taskflow.openstack.common import jsonutils from taskflow.openstack.common import jsonutils
from taskflow.openstack.common import uuidutils from taskflow.openstack.common import uuidutils
from taskflow.persistence import logbook
from taskflow import states from taskflow import states
from taskflow.utils import kazoo_utils from taskflow.utils import kazoo_utils
from taskflow.utils import lock_utils from taskflow.utils import lock_utils
@@ -145,17 +144,18 @@ class ZookeeperJob(base_job.Job):
def board(self): def board(self):
return self._board return self._board
def _load_book(self, book_uuid, book_name): def _load_book(self):
# No backend to attempt to fetch from :-( book_uuid = self.book_uuid
if self._backend is None: if self._backend is not None and book_uuid is not None:
return logbook.LogBook(name=book_name, uuid=book_uuid) # TODO(harlowja): we are currently limited by assuming that the
# TODO(harlowja): we are currently limited by assuming that the job # job posted has the same backend as this loader (to start this
# posted has the same backend as this loader (to start this seems to # seems to be a ok assumption, and can be adjusted in the future
# be a ok assumption, and can be adjusted in the future if we determine # if we determine there is a use-case for multi-backend loaders,
# there is a use-case for multi-backend loaders, aka a registry of # aka a registry of loaders).
# loaders). with contextlib.closing(self._backend.get_connection()) as conn:
with contextlib.closing(self._backend.get_connection()) as conn: return conn.get_logbook(book_uuid)
return conn.get_logbook(book_uuid) # No backend to fetch from or no uuid specified
return None
@property @property
def state(self): def state(self):
@@ -194,16 +194,27 @@ class ZookeeperJob(base_job.Job):
@property @property
def book(self): def book(self):
if self._book is None: if self._book is None:
loaded_book = None self._book = self._load_book()
try:
book_uuid = self._book_data['uuid']
book_name = self._book_data['name']
loaded_book = self._load_book(book_uuid, book_name)
except (KeyError, TypeError):
pass
self._book = loaded_book
return self._book return self._book
@property
def book_uuid(self):
if self._book:
return self._book.uuid
if self._book_data:
return self._book_data.get('uuid')
else:
return None
@property
def book_name(self):
if self._book:
return self._book.name
if self._book_data:
return self._book_data.get('name')
else:
return None
class ZookeeperJobBoardIterator(six.Iterator): class ZookeeperJobBoardIterator(six.Iterator):
"""Iterator over a zookeeper jobboard. """Iterator over a zookeeper jobboard.

View File

@@ -66,7 +66,24 @@ class Job(object):
@abc.abstractproperty @abc.abstractproperty
def book(self): def book(self):
"""Any logbook associated with this job.""" """Logbook associated with this job.
If no logbook is associated with this job, this property is None.
"""
@abc.abstractproperty
def book_uuid(self):
"""UUID of logbook associated with this job.
If no logbook is associated with this job, this property is None.
"""
@abc.abstractproperty
def book_name(self):
"""Name of logbook associated with this job.
If no logbook is associated with this job, this property is None.
"""
@property @property
def uuid(self): def uuid(self):

View File

@@ -253,6 +253,8 @@ class BoardTestMixin(object):
self.assertEqual(1, len(j.book)) self.assertEqual(1, len(j.book))
self.assertEqual(book.name, j.book.name) self.assertEqual(book.name, j.book.name)
self.assertEqual(book.uuid, j.book.uuid) self.assertEqual(book.uuid, j.book.uuid)
self.assertEqual(book.name, j.book_name)
self.assertEqual(book.uuid, j.book_uuid)
flow_details = list(j.book) flow_details = list(j.book)
self.assertEqual(flow_detail.uuid, flow_details[0].uuid) self.assertEqual(flow_detail.uuid, flow_details[0].uuid)