Contributor

fix #517

I'm not quite sure if I'm doing the right thing. 🤣

Please comment if I'm making mistake. 🗣

Member

It looks okay to me, did you do a manual test to see if it works for you?

Member

BTW, you also have to edit the requirements accordingly (at least for travis) to prevent this error:

/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/urllib3/contrib/socks.py:37: DependencyWarning: SOCKS support in urllib3 requires the installation of optional dependencies: specifically, PySocks.  For more information, see https://urllib3.readthedocs.io/en/latest/contrib.html#socks-proxies```

Contributor Author

@jh0ker Get it. Thanks for your tips.

Contributor Author

@jh0ker I did a manual test, and it works well.

Member

@daimajia that is expected behavior, no need for your to worry

Contributor Author

BTW. This issue

It looks all right for this PR. 😃

Member

It would be preferred to list the dependency as an extra dependency, similar to ujson. You'd have to put it in setup.py. To prevent import errors on systems where it is not installed, I recommend you move the import into the if that starts in line 97 of request.py

Contributor Author

Good suggestion. I'll finish it ASAP.

Contributor Author

@jh0ker Remove the PySocks dependency from requirements.txt, right? I'm not sure.

Member

@daimajia
yes, it is good. must have been my fault earlier. anyway i pushed my small changes and after Travis will finish unitests, I believe we'll be able to merge.

Member

@daimajia
sorry for the delay. the failure in CI is irrelevant to your changes, so I'm merging the PR.
Thanks for the good work.
BTW, I see that you haven't added yourself to AUTHORS.rst. If you'd like being mentioned there, feel free to send a second PR.

Contributor Author

@tsnoam Thanks, I'm pretty willing to add myself to the contributors list, I'm proud of contributing to this lib. 😊