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

Bulk-type arguments support #81

Closed
frol opened this issue Nov 13, 2015 · 7 comments

Comments

@frol
Copy link
Contributor

commented Nov 13, 2015

I'm not sure whether this case can be nicely handled with webargs, but anyway let me know if there is already an elegant solution to the issue I have.

I'm implementing a PATCH method in my RESTful API following RFC 6902, so the arguments would look like this:

PATCH /users/123

[
    { "op": "replace", "path": "/email", "value": "new.email@example.org" }
]

I have naively tried to pass many=True to my marshmallow schema, but it didn't work. Reading the code I couldn't find any hint that this might be a supported case.

Just for the sake of completeness, here is what was my naive attempt:

@use_args(MySchema(many=True))
def get(self):
    ...

I use Flask-RESTplus if this matters at all.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2015

The closest thing I could get is using a wrapping schema with Nested(many=True) field, but this adds unnecessary extra layer, i.e.:

{
    "body": [
        { "op": "replace", "path": "/email", "value": "new.email@example.org" }
    ]
}

instead of

[
    { "op": "replace", "path": "/email", "value": "new.email@example.org" }
]
@frol

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2015

@sloria Could you please advise me whether I should avoid such approach or I can improve webargs to handle this case out-of-the-box? I've read the source code and, unfortunatelly, I don't see a way to implement this case in the current design because now core.Parser.parse asks flaskparser.FlaskParser.parse_json for a specific field name, but I want to parse a list as a first level object and there is no name. Thus, it will require a noticable design change in parsing args.

NOTE: As a side effect we will be able to drop unnecessary parsed json caching from some backends since we will be able to parse all arguments in one pass.

@sloria

This comment has been minimized.

Copy link
Member

commented Nov 17, 2015

I think you present a valid use case and would gladly review a PR supporting it. Ideally, it would maintain backwards compatibility. I think we should be able to support

@use_args(MySchema(many=True), locations=('json', ))
def get(args):

without breaking compat, though I haven't given much thought to the implementation.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2015

@sloria I just want to let you know that I have managed to patch things up using custom Parser for this case and using virtual 'body' argument (the parser just returns full JSON if the field name is 'body'). It is not an elegant hack by any means, but it is the first step.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Nov 20, 2015

Here is my workaround described above (just for now): https://github.com/frol/flask-restplus-server-example/blob/master/flask_restplus_patched/parameters.py (Look at JSONParameters and JSONListFlaskParser classes)

@brettatoms

This comment has been minimized.

Copy link

commented Dec 27, 2015

+1

Unfortunately the best way to handle this is to use marshmallow directly:

operations, err = OperationSchema().load(request.get_json(), many=True)

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2016

Just to let everybody know, I'm working on this now. I have had a proof of concept, which I have successfully turned into a patch, but I'm also working on another improvement for apispec, so I will release make PRs shortly once I confirm they work for my project.

frol added a commit to frol/webargs that referenced this issue Feb 20, 2016

@sloria sloria closed this in d5d059f Apr 4, 2016

sloria added a commit that referenced this issue Apr 4, 2016

Document Bulk-type arguments and update changelog
Also fix incorrect examples

ref #81
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.