Skip to content
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

map/apply support in client/server #1504

Merged
merged 19 commits into from
May 18, 2016

Conversation

kwmsmith
Copy link
Member

@kwmsmith kwmsmith commented May 16, 2016

This picks up PR #1497 from @sandhujasmine:

  • creates the *_trusted variants for SerializationFormat instances.
  • Tests updated.
  • object_hook.py, json_dumps.py, and their _trusted variants created as well.
  • all _trusted dispatching is implemented as a superset of their underlying untrusted counterparts to keep it DRY.

kwmsmith and others added 14 commits April 27, 2016 13:17
- added json_dumps and object_hook for python builtin functions
- data_loads is not consistent for all serializations. For fastmsgpack,
  it uses pandas to decode to a Series; while other methods decode to a
  list.
- add most_formats frozen set to Serialization which includes all
  Serialization types except fastmsgpack. 'fastmsgpack' serializes/loads
  to a Series object as opposed to a list.

- break apart test_map_client_server() so all Serialization types that
  return a list are tested in one function. 'fastmsgpack' Serialzation
  is tested in test_map_client_server_fastmsgpack() since its assertion
  has to be for all() elements.
- checks the function given to map is pandas or numpy function. Raises
  NotImplementedError otherwise

- added simple tests for mapping numpy and pandas functions
- raise http exceptions if incoming json contains 'eval' or 'exec' which
  we don't support executing on server (ie 503 Forbidden)
- similarly if json contains a function not in pandas/numpy namespace,
  raise HTTP 501 Not Implemented exception

- don't limit what json_dumps can do since that is used for testing
- added tests but marked as xfail since pickle serialization is
  currently not raising these exceptions - need to ask/update
Create `*_trusted` versions of standard `SerializationFormat`s as well.
@kwmsmith kwmsmith added this to the 0.11 milestone May 16, 2016
@kwmsmith
Copy link
Member Author

@llllllllll @sandhujasmine you mind taking a once-over?

@llllllllll
Copy link
Member

I can take a look a little later today

from .json_dumps import json_dumps


json_dumps_trusted_ns = dict()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of the loop below this might be more clear as json_dumps_trusted_ns = jsun_dumps.funcs.copy()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I tried that initially, but the dispatching doesn't work in that case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how so, iterating over the dict and updating each k/v pair should be functionally equivalent? At the end of this module would the func maps be different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using json_dumps.funcs.copy() still retains the json_dumps function name, which ultimately triggers an exception in multipledispatch's inner dispatching logic; I have to do json_dumps_trusted.register(types)(func) to ensure the json_dumps_trusted function name is updated.

@llllllllll
Copy link
Member

Can we move the serialization code to a new package: blaze.server.serialization? I think this makes it easier to navigate to this code.


from .dispatch import dispatch
from functools import partial, wraps

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wraps appears to be an used import

@kwmsmith
Copy link
Member Author

Can we move the serialization code to a new package: blaze.server.serialization? I think this makes it easier to navigate to this code.

Good idea -- done.

@kwmsmith kwmsmith changed the title WIP: map/apply support in client/server map/apply support in client/server May 18, 2016
@kwmsmith kwmsmith removed the wip label May 18, 2016
@kwmsmith
Copy link
Member Author

@llllllllll @sandhujasmine I think this one is ready to merge.

@llllllllll
Copy link
Member

lgtm. Thanks!

@kwmsmith kwmsmith merged commit 85dbb05 into blaze:master May 18, 2016
@kwmsmith kwmsmith deleted the map-apply-client-server branch May 18, 2016 22:17
@kwmsmith kwmsmith modified the milestones: 0.11, 0.10.2 Jun 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants