-
Notifications
You must be signed in to change notification settings - Fork 390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow resources to be added dynamically to blaze server. #1329
Conversation
API question: A new data source is added to the severer by passing it a dictionary like And recall that a blaze server is either instantiated with no data, a single data source, or a dict of data sources. In the single data case, if I want to add Options:
I think option 1 is sensible. |
a8beb25
to
9889079
Compare
The server always serves a single resource, the dictionary case is not special it is just that the single resource is itself a record. I could see only supporting the update if the resource is a mutable mapping. |
Anything else y'all'd like to see? |
data_not_mm_msg = ("Cannot update blaze server data since its current data" | ||
" is a %s and not a mutable mapping (dictionary like).") | ||
if not isinstance(data, collections.MutableMapping): | ||
return (data_not_mm_msg % type(data), 400) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the right error code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a 422 Unprocessable Entity
would be better here since the syntax of the message is okay:
The 422 (Unprocessable Entity) status code means the server understands the content type of the request entity (hence a 415(Unsupported Media Type) status code is inappropriate), and the syntax of the request entity is correct (thus a 400 (Bad Request) status code is inappropriate) but was unable to process the contained instructions. For example, this error condition may occur if an XML request body contains well-formed (i.e., syntactically correct), but semantically erroneous, XML instructions.
Closes #1177 |
In #1177 @ywang007 mentions accessing data on the server URL's like http://localhost:6363/cities Does this work? It is not working for me locally in a web browser. |
There is no data endpoint like that in the default blaze blueprint. |
@@ -113,6 +114,32 @@ def authorized(*args, **kwargs): | |||
return authorized | |||
|
|||
|
|||
def check_request(f): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this stuff to a decorator since it was copied in the compserver
and addserver
functions.
@cowlicks LGTM, can you add a release note under |
@llllllllll any more feedback? |
ff7e1a7
to
8ac1a42
Compare
Merging soon |
@@ -25,6 +25,7 @@ Improved Backends | |||
|
|||
* Adds support for :func:`~blaze.expr.collections.tail` in the sql backend | |||
(:issue:`1289`). | |||
* Blaze Serever now supports dynamically adding datasets (:issue:`1329`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo here Serever
-> Server
@cowlicks can you add a doc blurb in the blaze server docs showing a minimal working example? |
status code 400 -> 422 refactored the request checking code copied in compute and add into a decorator.
@cpcloud the current example code with requests in the docs is not working for me. I'm working on it. |
8ac1a42
to
196d7c4
Compare
@@ -324,6 +324,22 @@ These queries deconstruct the Blaze expression as nested JSON. The ``":leaf"`` | |||
string is a special case pointing to the base data. Constructing these queries | |||
can be difficult to do by hand, fortunately Blaze can help you to build them. | |||
|
|||
Adding Data to the Server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpcloud doc blurb
Allow resources to be added dynamically to blaze server.
I'm still having some trouble with this, just getting it out there for now.
closes #1177