Skip to content

Conversation

@kasajei
Copy link

@kasajei kasajei commented Sep 22, 2014

Some service need POST params for the method.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8045b21 on kasajei:master into 08727f0 on requests:master.

Copy link
Member

Choose a reason for hiding this comment

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

Please rename to request_kwargs and a param line in the doc below

@ib-lundgren
Copy link
Member

Good idea :) See inline comments for a few nitpicks.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c5c7bf3 on kasajei:master into 08727f0 on requests:master.

@kasajei
Copy link
Author

kasajei commented Sep 23, 2014

@ib-lundgren Thank you for you comment.
I would not add kwargs but data for request, so refactored the parameter name.

@ib-lundgren
Copy link
Member

Ah I see. Still I think having the ability to add any keyword arg you may need would be useful and something I've been meaning to add for a while. Also, while data is currently a second position arg it is primarily a keyword arg and ideally we want to avoid depending on position.

Changing to

 oauth_sess.post(url, request_kwargs={'data': data})

is slightly more inconvenient than

 oauth_sess.post(url, request_data=data)

but allow users to use all of requests key words without us having to add them explicitly. For example changing headers and stream.

 headers = {'Accept': 'application/json'}
 oauth_sess.post(url, request_kwargs={'data': data, 'headers':headers, 'stream': True})

@Lukasa
Copy link
Member

Lukasa commented Feb 19, 2016

Closed in favour of #226.

@Lukasa Lukasa closed this Feb 19, 2016
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