From 4cdef3285dbec41ca939e4b4d35bd2268ae25213 Mon Sep 17 00:00:00 2001 From: Jim Paris Date: Mon, 18 Mar 2013 19:39:03 -0400 Subject: [PATCH] Destroy now requires that all data has been previously removed. Added new flag "-R" to command line to perform an automatic removal. This should be the last of the ways in which a single command could block the nilmdb thread for a long time. --- nilmdb/client/client.py | 2 +- nilmdb/cmdline/destroy.py | 14 +++++++++++--- nilmdb/server/nilmdb.py | 15 ++++++++++----- nilmdb/server/server.py | 2 +- tests/test_client.py | 10 +++++++++- tests/test_cmdline.py | 28 +++++++++++++++++++--------- 6 files changed, 51 insertions(+), 20 deletions(-) diff --git a/nilmdb/client/client.py b/nilmdb/client/client.py index eded86d..97295dd 100644 --- a/nilmdb/client/client.py +++ b/nilmdb/client/client.py @@ -97,7 +97,7 @@ class Client(object): return self.http.post("stream/create", params) def stream_destroy(self, path): - """Delete stream and its contents""" + """Delete stream. Fails if any data is still present.""" params = { "path": path } return self.http.post("stream/destroy", params) diff --git a/nilmdb/cmdline/destroy.py b/nilmdb/cmdline/destroy.py index 949ed38..ed6e8fb 100644 --- a/nilmdb/cmdline/destroy.py +++ b/nilmdb/cmdline/destroy.py @@ -7,11 +7,14 @@ def setup(self, sub): cmd = sub.add_parser("destroy", help="Delete a stream and all data", formatter_class = def_form, description=""" - Destroy the stream at the specified path. All - data and metadata related to the stream is - permanently deleted. + Destroy the stream at the specified path. + The stream must be empty. All metadata + related to the stream is permanently deleted. """) cmd.set_defaults(handler = cmd_destroy) + group = cmd.add_argument_group("Options") + group.add_argument("-R", "--remove", action="store_true", + help="Remove all data before destroying stream") group = cmd.add_argument_group("Required arguments") group.add_argument("path", help="Path of the stream to delete, e.g. /foo/bar", @@ -20,6 +23,11 @@ def setup(self, sub): def cmd_destroy(self): """Destroy stream""" + if self.args.remove: + try: + count = self.client.stream_remove(self.args.path) + except nilmdb.client.ClientError as e: + self.die("error removing data: %s", str(e)) try: self.client.stream_destroy(self.args.path) except nilmdb.client.ClientError as e: diff --git a/nilmdb/server/nilmdb.py b/nilmdb/server/nilmdb.py index 843ba21..e9bf392 100644 --- a/nilmdb/server/nilmdb.py +++ b/nilmdb/server/nilmdb.py @@ -450,17 +450,22 @@ class NilmDB(object): (newpath, stream_id)) def stream_destroy(self, path): - """Fully remove a table and all of its data from the database. - No way to undo it! Metadata is removed.""" + """Fully remove a table from the database. Fails if there are + any intervals data present; remove them first. Metadata is + also removed.""" stream_id = self._stream_id(path) - # Delete the cached interval data (if it was cached) + # Verify that no intervals are present, and clear the cache + iset = self._get_intervals(stream_id) + if len(iset): + raise NilmDBError("all intervals must be removed before " + "destroying a stream") self._get_intervals.cache_remove(self, stream_id) - # Delete the data + # Delete the bulkdata storage self.data.destroy(path) - # Delete metadata, stream, intervals + # Delete metadata, stream, intervals (should be none) with self.con as con: con.execute("DELETE FROM metadata WHERE stream_id=?", (stream_id,)) con.execute("DELETE FROM ranges WHERE stream_id=?", (stream_id,)) diff --git a/nilmdb/server/server.py b/nilmdb/server/server.py index 4f5294d..09a3540 100644 --- a/nilmdb/server/server.py +++ b/nilmdb/server/server.py @@ -210,7 +210,7 @@ class Stream(NilmApp): @exception_to_httperror(NilmDBError) @cherrypy.tools.CORS_allow(methods = ["POST"]) def destroy(self, path): - """Delete a stream and its associated data.""" + """Delete a stream. Fails if any data is still present.""" return self.db.stream_destroy(path) # /stream/rename?oldpath=/newton/prep&newpath=/newton/prep/1 diff --git a/tests/test_client.py b/tests/test_client.py index 4a15036..7ed20ff 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -375,6 +375,7 @@ class TestClient(object): # Delete streams that exist for stream in client.stream_list(): + client.stream_remove(stream[0]) client.stream_destroy(stream[0]) # Database is empty @@ -506,6 +507,10 @@ class TestClient(object): [ 109, 118 ], [ 200, 300 ] ]) + # destroy stream (try without removing data first) + with assert_raises(ClientError): + client.stream_destroy("/context/test") + client.stream_remove("/context/test") client.stream_destroy("/context/test") client.close() @@ -600,6 +605,7 @@ class TestClient(object): ]) # Clean up + client.stream_remove("/empty/test") client.stream_destroy("/empty/test") client.close() @@ -635,8 +641,9 @@ class TestClient(object): eq_(connections(), (1, 5)) # Clean up + c.stream_remove("/persist/test") c.stream_destroy("/persist/test") - eq_(connections(), (1, 6)) + eq_(connections(), (1, 7)) def test_client_13_timestamp_rounding(self): # Test potentially bad timestamps (due to floating point @@ -661,5 +668,6 @@ class TestClient(object): # Server will round this and give an error on finalize() ctx.insert("299999999.99 1\n") + client.stream_remove("/rounding/test") client.stream_destroy("/rounding/test") client.close() diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 4260f8f..2626b3d 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -716,6 +716,9 @@ class TestCmdline(object): self.fail("destroy /no/such/stream") self.contain("No stream at path") + self.fail("destroy -R /no/such/stream") + self.contain("No stream at path") + self.fail("destroy asdfasdf") self.contain("No stream at path") @@ -729,8 +732,14 @@ class TestCmdline(object): self.ok("list --detail") lines_(self.captured, 7) - # Delete some - self.ok("destroy /newton/prep") + # Fail to destroy because intervals still present + self.fail("destroy /newton/prep") + self.contain("all intervals must be removed") + self.ok("list --detail") + lines_(self.captured, 7) + + # Destroy for real + self.ok("destroy -R /newton/prep") self.ok("list") self.match("/newton/raw uint16_6\n" "/newton/zzz/rawnotch uint16_9\n") @@ -741,7 +750,8 @@ class TestCmdline(object): self.ok("destroy /newton/raw") self.ok("create /newton/raw uint16_6") - self.ok("destroy /newton/raw") + # Specify --remove with no data + self.ok("destroy --remove /newton/raw") self.ok("list") self.match("") @@ -816,7 +826,7 @@ class TestCmdline(object): # Now recreate the data one more time and make sure there are # fewer files. - self.ok("destroy /newton/prep") + self.ok("destroy --remove /newton/prep") self.fail("destroy /newton/prep") # already destroyed self.ok("create /newton/prep float32_8") os.environ['TZ'] = "UTC" @@ -827,7 +837,7 @@ class TestCmdline(object): for (dirpath, dirnames, filenames) in os.walk(testdb): nfiles += len(filenames) lt_(nfiles, 50) - self.ok("destroy /newton/prep") # destroy again + self.ok("destroy -R /newton/prep") # destroy again def test_14_remove_files(self): # Test BulkData's ability to remove when data is split into @@ -977,8 +987,8 @@ class TestCmdline(object): self.match("[ Thu, 01 Jan 2004 00:00:00.000000 +0000 -" "> Sat, 01 Jan 2005 00:00:00.000000 +0000 ]\n") - self.ok("destroy /diff/1") - self.ok("destroy /diff/2") + self.ok("destroy -R /diff/1") + self.ok("destroy -R /diff/2") def test_16_rename(self): # Test renaming. Force file size smaller so we get more files @@ -1042,7 +1052,7 @@ class TestCmdline(object): self.fail("rename /foo/bar /xxx/yyy/zzz/www") self.contain("path is subdir of existing node") self.ok("rename /foo/bar /xxx/yyy/mmm") - self.ok("destroy /xxx/yyy/zzz") + self.ok("destroy -R /xxx/yyy/zzz") check_path("xxx", "yyy", "mmm") # Extract it at the final path @@ -1050,7 +1060,7 @@ class TestCmdline(object): "--end '2012-03-23 10:04:01'") eq_(self.captured, extract_before) - self.ok("destroy /xxx/yyy/mmm") + self.ok("destroy -R /xxx/yyy/mmm") # Make sure temporary rename dirs weren't left around for (dirpath, dirnames, filenames) in os.walk(testdb):