search

Chaffelson / nipyapi

A convenient Python wrapper for Apache NiFi
Other
244 stars 76 forks source link

add socks proxy support #253

Closed Zyrix closed 3 years ago

Zyrix commented 3 years ago

This PR adds support for SOCKS proxies. It uses the "proxy" parameter for SOCKS proxies and distinguishes SOCKS proxies from other proxies by the occurence of "socks" in the proxy url. urllib3 is used for SOCKS proxies which uses pysocks for its implementation. See https://urllib3.readthedocs.io/en/latest/reference/contrib/socks.html

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.7%) to 69.11% when pulling 66446a0f0f7af2a7de3f86e99fce47110801f9ed on Zyrix:add-socks-proxy-support into d00221ba50bd83e21133d6e4d4b56741ead6822a on Chaffelson:main.

Chaffelson commented 3 years ago

Hi @Zyrix thanks for this contribution! It looks relatively straight forward, which is great, can I ask you to consider two things please. Firstly, is there some tests that can be easily integrated for this? Perhaps a socks proxy on a docker container could be added to our docker compose test environment, and appropriate tests run against that. Secondly, the client is generated from mustache templates, including the rest implementation. It would be good to include these changes in the template so that future clients do not drop them unexpectedly, you can find the template here: https://github.com/Chaffelson/nipyapi/blob/main/resources/client_gen/swagger_templates/rest.mustache

Both of these I am happy to help with if you like, please let me know your thoughts.

Zyrix commented 3 years ago

Thank you for your comments! I added my changes to the moustache template. As for the docker proxy test I'm not sure how to implement this and would be glad if you could help me with that.