Adding check on Content-Type header

mplanchard / falcon-marshmallow

Marshmallow serialization/deserialization middleware for Falcon

MIT License

4 stars 2 forks source link

Adding check on Content-Type header #14

Closed austin1howard closed 4 years ago

austin1howard commented 4 years ago

I have some endpoints which accept multiple types of content (json and pyarrow, in my case). When JSON is passed in I want to parse it with Marshmallow, but users can also access my webservice with a Python SDK which just encodes the raw objects as pyarrow, and I can skip the Marshmallow deserialization layer. I added a check on content type here.

If no content type is passed in, it assumes json (although who's submitting webservice requests with no content type?! tsk tsk)
If application/json or a variant (such as application/json;encoding=latin1) is passed, it will parse through Marshmallow
If another content type is passed, parsing will skip and nothing will be stored in req.context.

I also was seeing problems from dependencies missing which I cleaned up. If that broke something with tox or something else I don't know about then I'll revert that commit.

mplanchard commented 4 years ago

Thanks for this PR! I expect to have time to review this tonight, but at the latest I will look at it tomorrow. I agree with the general idea, though. I think passing through in cases where there's an unhandled content type makes a lot of sense

mplanchard commented 4 years ago

Also sorry for the CI delay here. I didn't realize GitLab CI didn't support the fork model (see https://gitlab.com/gitlab-org/gitlab/issues/5667). I'm working on getting a secondary CI provider up for PRs

austin1howard commented 4 years ago

ha, yeah I saw that. didn't know if you were considering switching to gitlab (my guess is forks work there, just not forks on GitHub since it's only mirroring this repo and not my forked repo). FWIW I love Drone as a CI solution, and they have a free-for-opensource cloud offering

mplanchard commented 4 years ago

Yeah, I used to use Travis for everything, but they fired all their senior developers a while back and I've been trying to switch. Nothing but pleased with GitLab so far up until this particular snag. You're right that it's just for GH-mirrored repos. I haven't looked into Drone, but I'll give that a look! I got Azure pipelines most of the way working last night, so I'll see if I can get that wrapped up real quick, too. Might as well catch 'em all 😄

mplanchard commented 4 years ago

I've got some work in progress to fix the minor linting error we're seeing and to add a couple of additional options. I'll open a PR against your PR with that. I'm writing tests now, but I'm not certain I'll finish tonight

austin1howard commented 4 years ago

Alright @mplanchard should be good to go here