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

Allow more complex content types. #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

philipn
Copy link
Contributor

@philipn philipn commented Feb 23, 2013

We use Content-type headers that look like this:

Content-Type: application/vnd.api.v1+json; charset=utf-8

and curently slumber barfs on these, refusing to decode. This change
allows you to specify, say, format='json' when initializing the API
object and deserialize properly.

I'm not sure if this is the totally right spot for this code, but I wanted
to bring this to ya'lls attention.

We use Content-type headers that look like this:

Content-Type: application/vnd.api.v1+json; charset=utf-8

and curently slumber barfs on these, refusing to decode.  This change
allows you to specify, say, format='json' when initializing the API
object and deserialize properly.

I'm not sure if this is the totally right spot for this code, but I wanted
to bring this to ya'lls attention.
stype = s.get_serializer()
try:
return stype.loads(resp.content)
except:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be at least except Exception

@moorepants
Copy link

@philipn This bug is a bit annoying for working with local wiki. I patched my slumber copy with this workaround, but should we make a better fix? If you are recommending slumber on the localwiki site for use, then it should be something that actually works.

@philipn
Copy link
Contributor Author

philipn commented Aug 17, 2013

We will be ditching the complex content types in LocalWiki API because of issues like this

On Aug 17, 2013, at 6:21 AM, Jason Moore [email protected] wrote:

@philipn This bug is a bit annoying for working with local wiki. I patched my slumber copy with this workaround, but should we make a better fix? If you are recommending slumber on the localwiki site for use, then it should be something that actually works.


Reply to this email directly or view it on GitHub.

@merwok
Copy link
Contributor

merwok commented Aug 19, 2013

Such content-types are really useful for many things, and this bug should be fixed. This PR just needs a small change (see my comment) and a test.

@EvaSDK
Copy link

EvaSDK commented Dec 29, 2014

Actually, if you have an API that returns Content-Type: text/plain but it actually returns UTF-8, it will return a badly (latin1) encoded string. Using the proper Content-Type: text/plain; charset=UTF-8 instead does not help because slumber just drops this information here: https://github.com/samgiles/slumber/blob/master/slumber/__init__.py#L115

While requests always returns a unicode object which is just as wrong in the first case, it at least says somewhere in the doc that it assumes things are unicode by default (iirc). And specifying the charset does make it work properly in this case.

So imho, this fix is not suited for fixing the problem completely though I don't know yet where to hook this exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants