Merge pull request blaze#1499 from kwmsmith/bugfix/server-add

Verify dataset is discoverable *before* adding it to server.

blaze/server/server.py

@@ -6,7 +6,6 @@
 import functools
 from hashlib import md5
 import os
-import re
 import socket
 from time import time
 from warnings import warn
@@ -643,10 +642,11 @@ def addserver(payload, serial):
         # before we can create the resource.
         for mod in imports:
             importlib.import_module(mod)
-        # Finally, we actually add the resource to the dataset
-        data.update({name: resource(source, *args, **kwargs)})
-        # Force discovery of new dataset to check that the data is loadable.
-        ds = discover(data)
+        # Make a new resource and try to discover it.
+        new_resource = {name: resource(source, *args, **kwargs)}
+        # Discovery is a minimal consistency check to determine if the new
+        # resource is valid.
+        ds = discover(new_resource)
         if name not in ds.dict:
             raise ValueError("%s not added." % name)
     except NotImplementedError as e:
@@ -657,5 +657,9 @@ def addserver(payload, serial):
         error_msg = "Addition failed with message:\n%s: %s"
         return (error_msg % (type(e).__name__, e),
                 RC.UNPROCESSABLE_ENTITY)
+    else:
+        # Now that we've established that the new resource is discoverable--and
+        # thus exists and is accessible--we add the resource to the server.
+        data.update(new_resource)
 
     return ('OK', RC.CREATED)

blaze/server/tests/test_server.py

@@ -5,7 +5,6 @@
 pytest.importorskip('flask.ext.cors')
 
 from base64 import b64encode
-from contextlib import contextmanager
 from copy import copy
 
 import datashape
@@ -96,6 +95,7 @@ def temp_server(request):
     with s.app.test_client() as c:
         yield c
 
+
 @pytest.yield_fixture(params=[None, tdata])
 def temp_add_server(request):
     """For when we want to mutate the server, and also add datasets to it."""
@@ -535,6 +535,26 @@ def test_isin(test, serial):
     assert resp['datashape'] == expected['datashape']
 
 
+@pytest.mark.parametrize('serial', all_formats)
+def test_add_errors(temp_add_server, serial):
+    pre_datashape = datashape.dshape(temp_add_server
+                                     .get('/datashape')
+                                     .data.decode('utf-8'))
+    bunk_path = example('bunk.csv')
+    blob = serial.dumps({'bunk': bunk_path})
+    response1 = temp_add_server.post('/add',
+                                     headers=mimetype(serial),
+                                     data=blob)
+    assert response1.status_code == RC.UNPROCESSABLE_ENTITY
+
+    # Test that the datashape of the server is accessible and unchanged after
+    # trying to add a non-existent dataset.
+    response2 = temp_add_server.get('/datashape')
+    assert response2.status_code == RC.OK
+    response_dshape = datashape.dshape(response2.data.decode('utf-8'))
+    assert_dshape_equal(pre_datashape, response_dshape)
+
+
 @pytest.mark.parametrize('serial', all_formats)
 def test_add_default_not_allowed(temp_server, serial):
     iris_path = example('iris.csv')

docs/source/whatsnew/0.10.1.txt

@@ -44,6 +44,8 @@ Bug Fixes
 
 * The content type specification parsing was improved to accept more elaborate
   headers (:issue:`1490`).
+* The discoverablility consistency check is done before a dataset is
+  dynamically added to the server (:issue:`1498`).
 
 Miscellaneous
 ~~~~~~~~~~~~~