Merge lp:~rconradharris/nova/backup_schedule_extension into lp:~hudson-openstack/nova/trunk

Proposed by Rick Harris
Status: Needs review
Proposed branch: lp:~rconradharris/nova/backup_schedule_extension
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 385 lines (+321/-7)
6 files modified
nova/api/openstack/contrib/backup_schedule.py (+271/-0)
nova/api/openstack/wsgi.py (+4/-2)
nova/tests/api/openstack/test_backup_schedule_extension.py (+44/-0)
nova/tests/api/openstack/test_extensions.py (+1/-0)
nova/tests/api/openstack/test_servers.py (+0/-5)
tools/pip-requires (+1/-0)
To merge this branch: bzr merge lp:~rconradharris/nova/backup_schedule_extension
Reviewer Review Type Date Requested Status
Brian Lamar (community) Needs Fixing
Review via email: mp+74665@code.launchpad.net

Description of the change

This patch adds a OpenStack API extension for handling the scheduling of backups.

As part of this effort, a new dependency was introduced on Tempo, the cron-as-a-service project.

To post a comment you must log in.
Revision history for this message
Brian Lamar (blamar) wrote :

130 + import tempo.client

Can we put this at the top of the file with the other imports. Unit tests are going to be run anyway so putting the import down here doesn't seem to do much. Alternatively I guess unit tests could be skipped if tempo isn't found?

review: Needs Fixing
Revision history for this message
Sandy Walsh (sandy-walsh) wrote :

Agreed ... it would be nice that, if tempo can't be imported it degrades gracefully (including the unit tests). Unless this is already the case?

Revision history for this message
Rick Harris (rconradharris) wrote :

> Can we put this at the top of the file with the other imports. Unit tests are going to be run anyway > so putting the import down here doesn't seem to do much.

Having `import tempo` in the `__init__` means that it won't be imported unless the Backup_schedule class is actually created.

Since the unit-tests don't actually construct the Backup_schedule class, this means `tempo` isn't required to run the tests.

This also means that `tempo` isn't required in a production environment unless the backup schedule extension is used.

Since API extensions are eager-loaded--everything in the 'contrib' directory is imported in search of an Extension class--I think it makes sense not to have 3rd party imports in the module-scope.

If we did, that would mean that users would be required to have every dependency of every (optional) extension. I don't think that scales.

Revision history for this message
Brian Lamar (blamar) wrote :

> Having `import tempo` in the `__init__` means that it won't be imported unless the Backup_schedule
> class is actually created.

Right, but we're putting Tempo in pip-requires and in the Ubuntu package right? So this should be considered a core extension (I'll attempt to not get started on the hilarity of that statement to me). If it were an 'optional extension' it would be packaged separately to manage dependencies.

> Since API extensions are eager-loaded--everything in the 'contrib' directory is imported in search
> of an Extension class--I think it makes sense not to have 3rd party imports in the module-scope.

This extension is in the base source code checkout, so if I checkout the Nova source, and do ./run_tests.sh it's going to require tempo, right? If so it's a requirement of the project unless we're up front about it and make the tests skip if tempo isn't installed.

> If we did, that would mean that users would be required to have every dependency of every
> (optional) extension. I don't think that scales.

No argument here. I really don't want to have tempo in pip-requires or the Ubuntu package if I'm not going to use it. Since it's there (or going to be there?), shouldn't we just show the imports up front?

My main argument though is that imports should always be up top in my opinion, even if you have to do something like:

try:
    import tempo
except ImportError:
    LOG.warn("Tempo not installed, backup extension won't work.")

Revision history for this message
Jay Pipes (jaypipes) wrote :

> > Having `import tempo` in the `__init__` means that it won't be imported
> unless the Backup_schedule
> > class is actually created.
>
> Right, but we're putting Tempo in pip-requires and in the Ubuntu package
> right? So this should be considered a core extension (I'll attempt to not get
> started on the hilarity of that statement to me).

Indeed, which is why I think the term "extension" has been a little bit abused in Nova ;)

> If it were an 'optional
> extension' it would be packaged separately to manage dependencies.

Indeed.

> > Since API extensions are eager-loaded--everything in the 'contrib' directory
> is imported in search
> > of an Extension class--I think it makes sense not to have 3rd party imports
> in the module-scope.
>
> This extension is in the base source code checkout, so if I checkout the Nova
> source, and do ./run_tests.sh it's going to require tempo, right? If so it's a
> requirement of the project unless we're up front about it and make the tests
> skip if tempo isn't installed.

++

> > If we did, that would mean that users would be required to have every
> dependency of every
> > (optional) extension. I don't think that scales.
>
> No argument here. I really don't want to have tempo in pip-requires or the
> Ubuntu package if I'm not going to use it. Since it's there (or going to be
> there?), shouldn't we just show the imports up front?
>
> My main argument though is that imports should always be up top in my opinion,
> even if you have to do something like:
>
> try:
> import tempo
> except ImportError:
> LOG.warn("Tempo not installed, backup extension won't work.")

Agree fully.
-jay

Revision history for this message
Rick Harris (rconradharris) wrote :

Thanks for the comments guys.

> > > Having `import tempo` in the `__init__` means that it won't be imported
> > unless the Backup_schedule
> > > class is actually created.
> >
> > Right, but we're putting Tempo in pip-requires and in the Ubuntu package
> > right? So this should be considered a core extension (I'll attempt to not
> get
> > started on the hilarity of that statement to me).

Good points here. Technically, `tempo` doesn't need to be in the pip-requires since, as mentioned above, the Schedule class isn't actually constructed (yet).

I reflexively added it to the `pip-requires` not for the venv-construction, but really just to indicate a dependency.

Perhaps we should separate out core-dependencies vs. optionals ones (as you allude to): maybe pip-optional-requires (ignore the oxymoron there ;-).

All that said, I see your point and agree.

> Indeed, which is why I think the term "extension" has been a little bit abused
> in Nova ;)
>
> > If it were an 'optional
> > extension' it would be packaged separately to manage dependencies.
>
> Indeed.

Yep, see above.

> > > Since API extensions are eager-loaded--everything in the 'contrib'
> directory
> > is imported in search
> > > of an Extension class--I think it makes sense not to have 3rd party
> imports
> > in the module-scope.
> >
> > This extension is in the base source code checkout, so if I checkout the
> Nova
> > source, and do ./run_tests.sh it's going to require tempo, right? If so it's
> a
> > requirement of the project unless we're up front about it and make the tests
> > skip if tempo isn't installed.
>
> ++
>
> > > If we did, that would mean that users would be required to have every
> > dependency of every
> > > (optional) extension. I don't think that scales.
> >
> > No argument here. I really don't want to have tempo in pip-requires or the
> > Ubuntu package if I'm not going to use it. Since it's there (or going to be
> > there?), shouldn't we just show the imports up front?
> >
> > My main argument though is that imports should always be up top in my
> opinion,
> > even if you have to do something like:
> >
> > try:
> > import tempo
> > except ImportError:
> > LOG.warn("Tempo not installed, backup extension won't work.")
>
> Agree fully.
> -jay

Yeah, I personally can go either way on that; but since HACKING has an opinion on this, you guys are right, this should go up at the top. I'll make that change.

Revision history for this message
Brian Lamar (blamar) wrote :

35 + import tempo2.client

Is it tempo.client or tempo2.client?

34 +try:
35 + import tempo2.client
36 + tempo_available = True
37 +except ImportError:
38 + tempo_available = False
39 + LOG.warn(_("Unable to import tempo, backup schedules API extension will"
40 + " not be available."))

137 + if tempo_available:
138 + self.tempo_client = tempo.client.TempoClient(FLAGS.tempo_url)

Looks good, however if tempo isn't installed, tempo_available = False, and the extension is loaded anyway the error most likely will occur as a AttributeError indicating that self.tempo_client isn't defined.

Might I suggest:

137 + if tempo_available:
138 + self.tempo_client = tempo.client.TempoClient(FLAGS.tempo_url)
139 + else:
140 + raise ImportError("tempo.client unable to be imported.")

Or alternatively:

try:
    import tempo.client as tempo_client
except ImportError:
    tempo_client = None

...

if tempo_client:
    self.tempo_client = tempo_client.TempoClient(FLAGS.tempo_url)
else:
    raise ImportError("tempo.client unable to be imported.")

25 +from nova import compute
134 + self.compute_api = compute.API()

I think these can be removed.

Looks great otherwise, sure we can get this in soon.

Revision history for this message
Jay Pipes (jaypipes) wrote :

Personally, I don't think that code should be added to Nova that depends on half-baked projects. And half-baked projects that have a bunch of new dependencies, like "requiem" and "flask". If tempo is going to be an OpenStack project, it already is going down the path of wildly divergent code paths for handling stuff that multiple OpenStack projects already handle in a fairly common fashion. Just sayin -- heading further and further away from common coding style and policies.

Revision history for this message
Rick Harris (rconradharris) wrote :

blamar: Good points; I'll fix those up shortly.

jaypipes:

> I don't think that code should be added to Nova that depends on half-baked projects.

Agree that Nova *core* should only depend on stable/mature projects. For extensions, I think we can relax those requirements and allow much more flexibility and freedom to experiment.

The problem as it stand now, though, is that Nova doesn't really have a notion of a completely optional extension. As Nova goes to production and we start integrating it with more and more custom systems, I think we're going to need to really think hard about how to do Nova extensions right, from a packaging, unit-test, and virt/compute/api layer perspective.

Perhaps we can get this discussion started at the Essex design summit?

Revision history for this message
Jay Pipes (jaypipes) wrote :

> The problem as it stand now, though, is that Nova doesn't really have a notion
> of a completely optional extension. As Nova goes to production and we start
> integrating it with more and more custom systems, I think we're going to need
> to really think hard about how to do Nova extensions right, from a packaging,
> unit-test, and virt/compute/api layer perspective.
>
> Perhaps we can get this discussion started at the Essex design summit?

Yes, definitely :) You're absolutely right that Nova doesn't currently support completely optional extensions. That's the root of the problems.

-jay

Revision history for this message
Matt Dietz (cerberus) wrote :

Code looks good

Seems like something that might make people happy is if we load tempo the way we load a lot of the drivers, which is by flags. Then any service that wants to do something cron-like would have to provide the same API. The default/base driver effectively would do nothing, and then we don't need to explicitly reference Tempo in any way in the project

1535. By Rick Harris

Merging trunk

1536. By Rick Harris

Raising if tempo.client unavailable

Revision history for this message
Rick Harris (rconradharris) wrote :

Thanks for the comments all-- I've updated the patch to reflect the commen

Matt: Yeah, the FLAGS+plugin model has served us really well, don't see a reason why it couldn't work here as well (eventually). As it stands now, though, OS API extensions have their own architecture, and it's probably better--for the time being at least--to be consistent.

This is definitely something we should discuss at the Essex summit: how to rework our extension architecture so it can support truly optional extensions at all layers of the code.

Unmerged revisions

1536. By Rick Harris

Raising if tempo.client unavailable

1535. By Rick Harris

Merging trunk

1534. By Rick Harris

Merging trunk

1533. By Rick Harris

Moving import to top of file

1532. By Rick Harris

Merging trunk

1531. By Rick Harris

Handling XML in the request

1530. By Rick Harris

Merging changes

1529. By Rick Harris

Merging in changes

1528. By Rick Harris

Merging trunk

1527. By Rick Harris

Removing test that is no longer relevant

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'nova/api/openstack/contrib/backup_schedule.py'
2--- nova/api/openstack/contrib/backup_schedule.py 1970-01-01 00:00:00 +0000
3+++ nova/api/openstack/contrib/backup_schedule.py 2011-09-21 02:40:26 +0000
4@@ -0,0 +1,271 @@
5+# Copyright 2011 Openstack, LLC.
6+#
7+# Licensed under the Apache License, Version 2.0 (the "License"); you may
8+# not use this file except in compliance with the License. You may obtain
9+# a copy of the License at
10+#
11+# http://www.apache.org/licenses/LICENSE-2.0
12+#
13+# Unless required by applicable law or agreed to in writing, software
14+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
15+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
16+# License for the specific language governing permissions and limitations
17+# under the License.
18+
19+"""The backup schedule extension."""
20+
21+import re
22+import webob
23+from webob import exc
24+
25+from nova import flags
26+from nova import log as logging
27+from nova.api.openstack import extensions as exts
28+from nova.api.openstack import faults
29+from nova.api.openstack import wsgi
30+
31+LOG = logging.getLogger("nova.api.contrib.backup_schedule")
32+
33+try:
34+ import tempo.client
35+ tempo_available = True
36+except ImportError:
37+ tempo_available = False
38+ LOG.warn(_("Unable to import tempo, backup schedules API extension will"
39+ " not be available."))
40+
41+FLAGS = flags.FLAGS
42+
43+# NOTE(sirp): This guard is needed because of an interaction between the way
44+# we define flags and the way we import API extensions.
45+#
46+# Flags are defined on import and may only be defined once (unless
47+# allow_override=True). Extensions are imported (via imp.load_source) each
48+# time a test is run, causing a DuplicateFlag error. Until we a) fix how
49+# extensions are imported and b) fix FLAGS leaking across tests (not just
50+# values changing but *which* flags can be seen), we'll need this guard.
51+if 'tempo_url' not in FLAGS:
52+ flags.DEFINE_string('tempo_url', 'http://127.0.0.1:8080',
53+ 'URL to pass to connect to Tempo with.')
54+
55+ flags.DEFINE_integer('weekly_backup_start_hour', 0,
56+ 'Hour that weekly backups should start (0..23)')
57+
58+ flags.DEFINE_integer('weekly_backup_start_minute', 0,
59+ 'Minutes that weekly backups should start on (0..59)')
60+
61+ flags.DEFINE_integer('daily_backup_start_minute', 0,
62+ 'Minutes that daily backups should start on (0..59)')
63+
64+RE_INSTANCE_UUID = re.compile('.*servers\/(.*)\/backup_schedule')
65+
66+DAYS = ['sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday',
67+ 'saturday']
68+
69+
70+def parse_day_str(day_str):
71+ """Return the day of the week given a day as a string.
72+
73+ Ex. Sunday -> 0, Monday -> 1, ...
74+ """
75+ day_of_week = DAYS.index(day_str.lower())
76+ return day_of_week
77+
78+
79+def parse_hour_spec(hour_spec):
80+ """Hour spec is a two-hour time-window that dictates when an hourly backup
81+ should kick off.
82+
83+ Ex. h_0000_0200 is a backup off that kicks off sometime between midnight
84+ and 2am GMT.
85+ """
86+ prefix, start_window, end_window = hour_spec.split('_')
87+ start_hour = int(start_window) / 100
88+ return start_hour
89+
90+
91+def make_hour_spec(start_hour):
92+ """Return an hour_spec given the start_hour.
93+
94+ Ex. 0 -> H_0000_2000, 22 -> H_2200_0000
95+ """
96+ if not (0 <= start_hour <= 22):
97+ raise ValueError("start hour must be between 0 and 22 inclusive")
98+ elif start_hour % 2 != 0:
99+ raise ValueError("must start on an even hour")
100+
101+ prefix = 'h'
102+ end_hour = (start_hour + 2) % 24
103+ hour_spec = "_".join([prefix, "%02d00" % start_hour,
104+ "%02d00" % end_hour])
105+ return hour_spec.upper()
106+
107+
108+def make_recurrence(minute, hour, day_of_week):
109+ """Return abbreviated form of recurrence.
110+
111+ Rather than use a full crontab entry, Tempo currently uses an abbreviated
112+ form of specifying the cron schedule, skipping the day of month and month
113+ fields.
114+ """
115+ return ' '.join(map(str, [minute, hour, day_of_week]))
116+
117+
118+def wrap_errors(fn):
119+ """"Ensure errors are not passed along."""
120+ def wrapped(*args):
121+ try:
122+ return fn(*args)
123+ except Exception as e:
124+ LOG.error(e)
125+ return faults.Fault(exc.HTTPInternalServerError())
126+ return wrapped
127+
128+
129+class Backup_schedule(exts.ExtensionDescriptor):
130+ """The Rescue controller for the OpenStack API."""
131+ def __init__(self):
132+ super(Backup_schedule, self).__init__()
133+ self.serializer = wsgi.ResponseSerializer()
134+ self.deserializer = wsgi.RequestDeserializer()
135+ if tempo_available:
136+ self.tempo_client = tempo.client.TempoClient(FLAGS.tempo_url)
137+ else:
138+ raise ImportError(_("tempo.client unable to be imported."))
139+
140+ def _get_instance_uuid(self, req):
141+ instance_uuid = RE_INSTANCE_UUID.match(req.path_info).group(1)
142+ return instance_uuid
143+
144+ @wrap_errors
145+ def _get_backup_schedule(self, req, res):
146+ """Return a backup schedule for server."""
147+ instance_uuid = self._get_instance_uuid(req)
148+
149+ weekly = None
150+ daily = None
151+ for task in self._get_tempo_backups_for_instance(instance_uuid):
152+ minute, hour, day_of_week = task['recurrence'].split(' ')
153+ is_daily = day_of_week == '*'
154+ if is_daily:
155+ start_hour = int(hour)
156+ daily = make_hour_spec(start_hour)
157+ else:
158+ weekly = DAYS[int(day_of_week)].upper()
159+
160+ response_data = {
161+ "backupSchedule": {
162+ "enabled": any([daily, weekly]),
163+ "weekly": weekly or 'DISABLED',
164+ "daily": daily or 'DISABLED'
165+ }
166+ }
167+
168+ content_type = self._pick_response_content_type(req)
169+ self.serializer.serialize(
170+ response_data, content_type, response=res)
171+ return res
172+
173+ def _pick_response_content_type(self, req):
174+ """Determine the content_type to respond with, either JSON or XML.
175+
176+ XML is returned when the client specifically asks for it; otherwise
177+ JSON is returned.
178+ """
179+ if ('Content-Type' in req.headers and
180+ req.headers['Content-Type'].lower() == 'application/xml'):
181+ return 'application/xml'
182+
183+ return 'application/json'
184+
185+ def _get_tempo_backups_for_instance(self, instance_uuid):
186+ # FIXME(sirp): When Tempo exposes a more efficient mechanism for
187+ # grabbing Tasks related to an instance, we should use that; until
188+ # then, just grab all tasks and filter by instance uuid here.
189+ for task in self.tempo_client.task_get_all():
190+ is_backup = task['task'] in ('daily_backup', 'weekly_backup')
191+ if task['instance_uuid'] == instance_uuid and is_backup:
192+ yield task
193+
194+ def _disable_backups(self, instance_uuid):
195+ """Disable backups for a given instance."""
196+ for task in self._get_tempo_backups_for_instance(instance_uuid):
197+ LOG.debug(_("Deleting backup for instance '%(instance_uuid)s',"
198+ " Tempo task %(task)s") % locals())
199+ self.tempo_client.task_delete(task['uuid'])
200+
201+ def _enable_weekly_backup(self, instance_uuid, day_str):
202+ """Enable a weekly backup for the given instance. """
203+ day_of_week = parse_day_str(day_str)
204+ recurrence = make_recurrence(
205+ minute=FLAGS.weekly_backup_start_minute,
206+ hour=FLAGS.weekly_backup_start_hour,
207+ day_of_week=day_of_week)
208+
209+ LOG.debug(_("Creating weekly backup for instance '%s'"), instance_uuid)
210+ task = self.tempo_client.task_create(
211+ 'weekly_backup', instance_uuid, recurrence)
212+ LOG.debug(_("Created Tempo task '%s'"), task)
213+
214+ def _enable_daily_backup(self, instance_uuid, hour_spec):
215+ """Enable a daily backup for the given instance. """
216+ start_hour = parse_hour_spec(hour_spec)
217+ recurrence = make_recurrence(
218+ minute=FLAGS.daily_backup_start_minute,
219+ hour=start_hour,
220+ day_of_week='*')
221+ LOG.debug(_("Creating daily backup for instance '%s'"), instance_uuid)
222+ task = self.tempo_client.task_create(
223+ 'daily_backup', instance_uuid, recurrence)
224+ LOG.debug(_("Created Tempo task '%s'"), task)
225+
226+ @wrap_errors
227+ def _post_backup_schedule(self, req, res):
228+ """Create or update a backup schedule for server."""
229+ instance_uuid = self._get_instance_uuid(req)
230+ self._disable_backups(instance_uuid)
231+
232+ schedule = self.deserializer.deserialize_body(
233+ req, 'default')['body']['backupSchedule']
234+
235+ if schedule['enabled']:
236+ day_str = schedule.get('weekly', 'disabled').lower()
237+ if day_str != 'disabled':
238+ self._enable_weekly_backup(instance_uuid, day_str)
239+
240+ hour_spec = schedule.get('daily', 'disabled').lower()
241+ if hour_spec != 'disabled':
242+ self._enable_daily_backup(instance_uuid, hour_spec)
243+
244+ return webob.Response(status_int=204)
245+
246+ @wrap_errors
247+ def _delete_backup_schedule(self, req, res):
248+ """Delete a backup schedule for server."""
249+ instance_uuid = self._get_instance_uuid(req)
250+ self._disable_backups(instance_uuid)
251+ return webob.Response(status_int=204)
252+
253+ def get_name(self):
254+ return "Backup_schedule"
255+
256+ def get_alias(self):
257+ return "os-backup-schedule"
258+
259+ def get_description(self):
260+ return "Instance backup schedule"
261+
262+ def get_namespace(self):
263+ return "http://docs.openstack.org/ext/backup-schedule/api/v1.1"
264+
265+ def get_updated(self):
266+ return "2011-08-18T00:00:00+00:00"
267+
268+ def get_request_extensions(self):
269+ url = '/:(project_id)/servers/:(id)/backup_schedule'
270+ extensions = [
271+ exts.RequestExtension('GET', url, self._get_backup_schedule),
272+ exts.RequestExtension('POST', url, self._post_backup_schedule),
273+ exts.RequestExtension('DELETE', url, self._delete_backup_schedule)
274+ ]
275+ return extensions
276
277=== modified file 'nova/api/openstack/wsgi.py'
278--- nova/api/openstack/wsgi.py 2011-09-15 18:32:10 +0000
279+++ nova/api/openstack/wsgi.py 2011-09-21 02:40:26 +0000
280@@ -453,14 +453,16 @@
281 self.headers_serializer = headers_serializer or \
282 ResponseHeadersSerializer()
283
284- def serialize(self, response_data, content_type, action='default'):
285+ def serialize(self, response_data, content_type, action='default',
286+ response=None):
287 """Serialize a dict into a string and wrap in a wsgi.Request object.
288
289 :param response_data: dict produced by the Controller
290 :param content_type: expected mimetype of serialized response body
291
292 """
293- response = webob.Response()
294+ if not response:
295+ response = webob.Response()
296 self.serialize_headers(response, response_data, action)
297 self.serialize_body(response, response_data, content_type, action)
298 return response
299
300=== added file 'nova/tests/api/openstack/test_backup_schedule_extension.py'
301--- nova/tests/api/openstack/test_backup_schedule_extension.py 1970-01-01 00:00:00 +0000
302+++ nova/tests/api/openstack/test_backup_schedule_extension.py 2011-09-21 02:40:26 +0000
303@@ -0,0 +1,44 @@
304+# vim: tabstop=4 shiftwidth=4 softtabstop=4
305+#
306+# Copyright 2011 Openstack, LLC.
307+#
308+# Licensed under the Apache License, Version 2.0 (the "License"); you may
309+# not use this file except in compliance with the License. You may obtain
310+# a copy of the License at
311+#
312+# http://www.apache.org/licenses/LICENSE-2.0
313+#
314+# Unless required by applicable law or agreed to in writing, software
315+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
316+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
317+# License for the specific language governing permissions and limitations
318+# under the License.
319+
320+"""Unit tests for the Backup Scheduler Extension"""
321+
322+from nova import test
323+from nova.api.openstack.contrib import backup_schedule
324+
325+
326+class TestMakeHourSpec(test.TestCase):
327+ def test_start_hour_is_0(self):
328+ start_hour = 0
329+ expected = "H_0000_0200"
330+ result = backup_schedule.make_hour_spec(start_hour)
331+ self.assertEquals(expected, result)
332+
333+ def test_start_hour_is_22(self):
334+ start_hour = 22
335+ expected = "H_2200_0000"
336+ result = backup_schedule.make_hour_spec(start_hour)
337+ self.assertEquals(expected, result)
338+
339+ def test_start_hour_is_1(self):
340+ start_hour = 1
341+ self.assertRaises(
342+ ValueError, backup_schedule.make_hour_spec, start_hour)
343+
344+ def test_start_hour_is_24(self):
345+ start_hour = 24
346+ self.assertRaises(
347+ ValueError, backup_schedule.make_hour_spec, start_hour)
348
349=== modified file 'nova/tests/api/openstack/test_extensions.py'
350--- nova/tests/api/openstack/test_extensions.py 2011-09-14 15:54:56 +0000
351+++ nova/tests/api/openstack/test_extensions.py 2011-09-21 02:40:26 +0000
352@@ -85,6 +85,7 @@
353 ext_path = os.path.join(os.path.dirname(__file__), "extensions")
354 self.flags(osapi_extensions_path=ext_path)
355 self.ext_list = [
356+ 'Backup_schedule',
357 "Createserverext",
358 "FlavorExtraSpecs",
359 "FlavorExtraData",
360
361=== modified file 'nova/tests/api/openstack/test_servers.py'
362--- nova/tests/api/openstack/test_servers.py 2011-09-19 20:16:49 +0000
363+++ nova/tests/api/openstack/test_servers.py 2011-09-21 02:40:26 +0000
364@@ -2424,11 +2424,6 @@
365 res = req.get_response(fakes.wsgi_app())
366 self.assertEqual(res.status_int, 501)
367
368- def test_server_backup_schedule_deprecated_v1_1(self):
369- req = webob.Request.blank('/v1.1/fake/servers/1/backup_schedule')
370- res = req.get_response(fakes.wsgi_app())
371- self.assertEqual(res.status_int, 404)
372-
373 def test_get_all_server_details_xml_v1_0(self):
374 req = webob.Request.blank('/v1.0/servers/detail')
375 req.headers['Accept'] = 'application/xml'
376
377=== modified file 'tools/pip-requires'
378--- tools/pip-requires 2011-09-02 05:39:31 +0000
379+++ tools/pip-requires 2011-09-21 02:40:26 +0000
380@@ -35,4 +35,5 @@
381 nosexcover
382 GitPython
383 paramiko
384+tempo
385 feedparser