x/gantt
Code Issues Proposed changes

Clean up read_deleted support in host aggregates code

Browse Source 
tl;dr - this is a cleaner fix for bug #949038

It seems clear to me that all of the DB APIs should not explicitly pass
read_deleted='no' or read_deleted='yes' to model_query without good
reason.

We want to allow callers to specify read_deleted via the context and
that only works if we don't explicitly pass it to model_query().

If we don't explicitly specify it to model_query(), we use the value
from the context which defaults to 'no'.

Given all that, there is no need to support read_deleted to any of the
DB API calls because they should support specifying the flag via the
context. There should also be no need to pass read_deleted='no' because
that is the default.

Really, the only place there should be any mention of read_deleted is
where we want read_deleted='yes' behaviour e.g.

  - In tests, where we want to check the operational_state of an
    aggregate after it has been deleted

  - Where we want to support undeleting an aggregate or aggreate host

Change-Id: I916a8d189a33d7f30838cccb26531a024066ef96
This commit is contained in:
Mark McLoughlin
2012-03-25 21:59:29 +01:00
parent db8bb2463e
commit 86759329a6
2 changed files with 8 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
nova/tests/test_compute.py
View File

@@ -3287,8 +3287,8 @@ class ComputeAPIAggrTestCase(test.TestCase):
aggr = self.api.create_aggregate(self.context, 'fake_aggregate',
'fake_zone')
self.api.delete_aggregate(self.context, aggr['id'])
expected = db.aggregate_get(self.context, aggr['id'],
read_deleted='yes')
expected = db.aggregate_get(self.context.elevated(read_deleted='yes'),
aggr['id'])
self.assertNotEqual(aggr['operational_state'],
expected['operational_state'])

15
nova/tests/test_db_api.py
View File

@@ -434,11 +434,10 @@ class AggregateDBApiTestCase(test.TestCase):
ctxt = context.get_admin_context()
result = _create_aggregate(context=ctxt, metadata=None)
db.aggregate_delete(ctxt, result['id'])
expected = db.aggregate_get_all(ctxt, read_deleted='no')
expected = db.aggregate_get_all(ctxt)
self.assertEqual(0, len(expected))
ctxt = context.get_admin_context(read_deleted='yes')
aggregate = db.aggregate_get(ctxt, result['id'])
aggregate = db.aggregate_get(ctxt.elevated(read_deleted='yes'),
result['id'])
self.assertEqual(aggregate["operational_state"], "dismissed")
def test_aggregate_update(self):
@@ -506,7 +505,7 @@ class AggregateDBApiTestCase(test.TestCase):
values=values, metadata=None))
for c in xrange(1, remove_counter):
db.aggregate_delete(ctxt, aggregates[c - 1].id)
results = db.aggregate_get_all(ctxt, read_deleted='no')
results = db.aggregate_get_all(ctxt)
self.assertEqual(len(results), add_counter - remove_counter)
def test_aggregate_metadata_add(self):
@@ -564,8 +563,7 @@ class AggregateDBApiTestCase(test.TestCase):
host = _get_fake_aggr_hosts()[0]
db.aggregate_host_delete(ctxt, result.id, host)
db.aggregate_host_add(ctxt, result.id, host)
expected = db.aggregate_host_get_all(ctxt, result.id,
read_deleted='no')
expected = db.aggregate_host_get_all(ctxt, result.id)
self.assertEqual(len(expected), 1)
def test_aggregate_host_add_duplicate_raise_conflict(self):
@@ -602,8 +600,7 @@ class AggregateDBApiTestCase(test.TestCase):
result = _create_aggregate_with_hosts(context=ctxt, metadata=None)
db.aggregate_host_delete(ctxt, result.id,
_get_fake_aggr_hosts()[0])
expected = db.aggregate_host_get_all(ctxt, result.id,
read_deleted='no')
expected = db.aggregate_host_get_all(ctxt, result.id)
self.assertEqual(0, len(expected))
def test_aggregate_host_delete_raise_not_found(self):