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

Pass the serialization format as a mimetype #1176

Merged
merged 10 commits into from
Jul 14, 2015
Merged

Pass the serialization format as a mimetype #1176

merged 10 commits into from
Jul 14, 2015

Conversation

cpcloud
Copy link
Member

@cpcloud 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
Copy link
Member Author

cpcloud commented Jul 13, 2015

cc @llllllllll

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

yep

@llllllllll
Copy link
Member

Just one comment, but looks good

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch

Copy link
Member Author

Choose a reason for hiding this comment

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

if None should that also be a 415?

Copy link
Member

Choose a reason for hiding this comment

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

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 a commit that referenced this pull request Jul 14, 2015
Pass the serialization format as a mimetype
@cpcloud cpcloud merged commit dd56e3b into blaze:master Jul 14, 2015
@cpcloud cpcloud deleted the down-with-dot branch July 14, 2015 11:21
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.

None yet

2 participants