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

Pass the serialization format as a mimetype #1176

Merged
merged 10 commits into from Jul 14, 2015

Conversation

Projects
None yet
2 participants
@cpcloud
Member

cpcloud commented Jul 13, 2015

the serialization format json, msgpack, etc is now passed as a vendor specific mime type instead of being part of the URL:

requests.get('/compute',
             data=dict(expr=to_tree(expr)),
             headers={'Content-Type': 'application/vnd.blaze+json'})

@cpcloud cpcloud self-assigned this Jul 13, 2015

@cpcloud cpcloud added this to the 0.8.3 milestone Jul 13, 2015

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jul 13, 2015

@@ -130,8 +143,9 @@ def compute_down(expr, ec, **kwargs):
serial = ec.serial
r = post(
ec,
'compute.{serial}'.format(serial=serial.name),
'/compute',

This comment has been minimized.

@llllllllll

llllllllll Jul 13, 2015

Member

this will generate 2 slashes. we might want to update the _request function to not add the slash.

This comment has been minimized.

@cpcloud

cpcloud Jul 13, 2015

Member

fixed

try:
serial = _get_format(serial_format)
serial = _get_format(*re.match(r'^application/vnd\.blaze\+(.+)$',
content_type).groups())
except KeyError:
return 'Unsupported serialization format', 404

This comment has been minimized.

@llllllllll

llllllllll Jul 13, 2015

Member

This should be a 415 now

try:
serial = _get_format(serial_format)
serial = _get_format(*mimetype_regex.match(content_type).groups())

This comment has been minimized.

@llllllllll

llllllllll Jul 13, 2015

Member

The * makes me think that this could possibly be more than one value; could we just take [0] instead?

This comment has been minimized.

@cpcloud
@llllllllll

This comment has been minimized.

Member

llllllllll commented Jul 13, 2015

Just one comment, but looks good

try:
serial = _get_format(serial_format)
serial = _get_format(mimetype_regex.match(content_type).groups()[0])

This comment has been minimized.

@llllllllll

llllllllll Jul 13, 2015

Member

actually, this needs to check that match is not none before taking groups()[0] or we get 500 instead of 415

This comment has been minimized.

@cpcloud

cpcloud Jul 13, 2015

Member

good catch

This comment has been minimized.

@cpcloud

cpcloud Jul 13, 2015

Member

if None should that also be a 415?

This comment has been minimized.

@llllllllll

llllllllll Jul 13, 2015

Member

Yeah, None means we don't know what the media type is; however, a keyerror means we know what it is but this particular server doesn't use that format

cpcloud added some commits Jul 13, 2015

cpcloud added a commit that referenced this pull request Jul 14, 2015

Merge pull request #1176 from cpcloud/down-with-dot
Pass the serialization format as a mimetype

@cpcloud cpcloud merged commit dd56e3b into blaze:master Jul 14, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@cpcloud cpcloud deleted the cpcloud:down-with-dot branch Jul 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment