Skip to content

Conversation

vitteloil
Copy link
Contributor

Sites sections of praw.ini now handle the optional directive
'http_proxy'.
Example
[reddit.com]
http_proxy:127.0.0.1:8000

Available URI are the ones handled by the Requests python module.
Proxy authentication would work with:
http_proxy:user:password@host:port

All tests passing with and without proxy (regular tests as well as python setup.py test -s
praw.tests.OAuth2Test

Sites sections of praw.ini now handle the optional directive
'http_proxy'.
Example
[reddit.com]
http_proxy:127.0.0.1:8000

Available URI are the ones handled by the Requests python module.
Proxy authentication would work with:
http_proxy:user:password@host:port

All tests passing (regular tests as well as python setup.py test -s
praw.tests.OAuth2Test
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you've interrupted the log_requests documentation here. Please move it after.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@bboe
Copy link
Member

bboe commented Apr 5, 2013

Great addition overall. Please just make the necessary changes as indicated in-line. Thanks!

When I added http_proxy, I destroyed the log_requests configuration
explanation in the documentation. Fixed.
As suggested by Bryce Boe, the proxy dict creation for the Requests
request is now handled once, on the session creation, instead of being
created at each request.
Copy link
Member

Choose a reason for hiding this comment

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

I think this whitespace is necessary for the list to show properly, though I could be wrong. Please verify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I re-generated the html doc and it seems fine

2013/4/6 Bryce Boe notifications@github.com

In docs/pages/configuration_files.rst:

@@ -60,10 +60,10 @@ config file. Each site can overwrite any of these variables.
mappings are created dynamically on site creation and thus isn't consistent
across sites.

* log_requests A integer that determines the level of API call logging.

I think this whitespace is necessary for the list to show properly.


Reply to this email directly or view it on GitHubhttps://github.com//pull/206/files#r3681420
.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, cool.

@Damgaard
Copy link
Contributor

Damgaard commented Apr 6, 2013

This looks pretty cool. Be sure to update CHANGES.rst with this new feature.

@vitteloil
Copy link
Contributor Author

I created a 2.0.15 section and added the feature. Will it work for you ? I
didn't change the setup.py 's version string.

2013/4/6 Henri Godron enjoyaol@gmail.com

Hi Andreas, which version number should I add this improvement to ?

2013/4/6 Andreas Damgaard Pedersen notifications@github.com

This looks pretty cool. Be sure to update CHANGES.rst with this new
feature.


Reply to this email directly or view it on GitHubhttps://github.com//pull/206#issuecomment-15991594
.

@Damgaard
Copy link
Contributor

Damgaard commented Apr 6, 2013

My concern is to get it in the changelog, so people can notice your great addition. Bboe is the maintainer and decides on versinoning. I think the best thing would be to just leave it as it is now and he can decide what version to put it in and update the code accordingly when he squashes the commits.

@bboe
Copy link
Member

bboe commented Apr 6, 2013

Merged as c3120f5. Deployed as 2.0.15. Thanks!

@bboe bboe closed this Apr 6, 2013
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.

3 participants