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

Source and target parameters are confusing #32

Closed
joshfriend opened this issue Feb 3, 2015 · 4 comments

Comments

@joshfriend
Copy link

commented Feb 3, 2015

I was expecting the target argument to specify the location in the parsed set where the argument value would be stored (similar to stdlib argparse's or flask-restful's dest kwarg).
I also thought that source was the location in the request to look for the argument (see source arg from flask-restful).

Due to the pending deprecation of the reqparse module of Flask-Restful (flask-restful/flask-restful#335), I think swapping these two arguments would help migration (I've been referring people to this package to use instead of reqparse) and general ease of use.

@yani-

This comment has been minimized.

Copy link

commented Feb 4, 2015

+1

@sloria

This comment has been minimized.

Copy link
Member

commented Feb 5, 2015

I will have to think about this. It would be nice if we could do this while maintaining at least some backwards-compatibility. Switching the names would change the semantics in a significant way, such that the Arg objects that used the source argument would have to reverse the keys and values.

# current
args = {
    'content_type': Arg(str, source='Content-Type'),
}

# if target and source are switched
args = {
    'Content-Type': Arg(str, target='content_type')
}

We might be able to reclaim some backwards-compat if we add a dest parameter, which would be the key for the parsed value in the output dict (instead of target in the example above). source could then be deprecated. We could also add a src parameter and deprecate target.

# backwards-compatible solution
args = {
    'Content-Type': Arg(str, dest='content_type', src='headers')
}

I appreciate that Flask-RESTful is supporting webargs; I would be happy to help with the migration in any way I can.

UPDATE: Add code example for possible solution.

@sloria

This comment has been minimized.

Copy link
Member

commented Mar 2, 2015

I've added the dest parameter as described above and renamed targets to locations. Hopefully this clears things up.

@sloria sloria closed this in #33 Mar 2, 2015

@joshfriend

This comment has been minimized.

Copy link
Author

commented Mar 2, 2015

Yeah, I think that helps clear it up nicely. Thanks!

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.