lczub / TestLink-API-Python-client

A Python client to use the TestLink API
105 stars 64 forks source link

Proxy configuration support in TestLinkHelper #36

Closed Maberi closed 9 years ago

Maberi commented 9 years ago

This patch allows easy proxy configuration using TestLinkHelper.

Adds a new --proxy option in command line. Recognizes "http_proxy" environment variable.

lczub commented 9 years ago

Hello Mario,

thanks for your suggestion to support proxy transport. Impression of a short patch review is, that this is a project solution and it will require some rework cycles to get a product solution.

Here are some of my open points / questions:

a) origin of class ProxiedTransport it seams, that parts of ProxiedTransport are a copy of PY27 examples or an existing implementation. please add a class comment, which origin do you use (or which part is from you and which from others) is it maybe possible to import / install an existing implementation instead of copy & paste existing code?

b) separate source file ProxiedTransport I prefer to define only one class per file. so please separate ProxiedTransport into a separate source file. this make it clearer, which import is needed for which class file and class should get a prefix like "testlink..."

c) hard coded environment variable 'http_proxy' please use a class variable like ENVNAME_PROXY and TL specific env variable like TESTLINK_API_PYTHON_PROXY this make it easier for subclasses to use different variables and avoid conflicts, if customers environment has defined a 'http_proxy' env variable, which is not relevant for testlink

d) py3 support since yesterday and pull request #33 a new branch py3 exist with lots of adjustments by manjoklm to make the code runnable under py33 and py34. This could be the base for the next release. So it would be helpful, if your work already includes these changes. Could you work against the py3 branch?

e) tests we need some testlinkhelper tests for the new command line argument --proxy and new environment variable. could you extend utest-offline\testlinkhelpertest and define also a new test class for the ProxiedTransport?

f) doc - example new command line argument means change in users interface, some proxy examples in doc/usage.rst would be helpful. Your code make a decision "if self.accept_gzip_encoding" - must the customer implement additional packages like gzip? if so, this should be documented, too.

g) http vs https I'm not familiar with proxy and transport, but your code seams to handle only httplib.HTTPConnection and no httplib.HTTPSConnection. When thinking about a proxy support, mustn't be HTTPS supported , too?

Sorry many questions. Thats the reason why I can not integrate your first version. Our scope should be to create a stabil solution. Is this ok?

Greetings Luiko

Maberi commented 9 years ago

Hello, Luiko:

It's my first contribution to a open source project via Pull Request. I've have had to modify the module for my work needs and I wanted to offer this modifications to the project, since proxy support is something not easy to guess.

Answering your questions:

a) The code for ProxiedTransport is new, but not from scratch. I've combined the sample of using proxy with xmlrpclib and portions of code from the same xmlrpclib.

b) Since it is part from a "dirty" helper class and not essential part of the client (you may instantiate the client without the helper class and you may supply your own transport), I left it there as a quick and simple solution. The name is derived from its parent class and its functions have nothing to do with the Teslink interface by its own. So I think the name is fine this way.

c) Environment variable "http_proxy" is a de facto standard for proxy definition via environment variables. For example: http://ubuntuforums.org/showthread.php?t=1575

d) Yeah, it'll be great. I prefer py3, but I had to develop my program in py2 because this module did not support py3.

e) Complicated because my own availability.

f) Same as e. I've read gzip is an optional package in building Python. It is an optional import. The original code was this way.

g) I've had an http proxy for develop and test but, same as e, it's complicated to have time to configure and test with an https proxy.

Conclusion: because of (e), and since programming Python is very far away from my job duties, it's very complicated I could enhance this pull request much beyond where it is now. I'm sorry but I must keep some time to sleep :)

Regards, Mario

lczub commented 9 years ago

Hello Mario, your offer is welcome and I understand your project time pressing restrictions . As first step I will pull your patch into a separate branch , so your work will be saved and visible for other developer. As second step, after some small tests, I will try try to publish a prereleales under https://github.com/lczub/TestLink-API-Python-client/releases as it is, so other users, which need proxy support immediately could use it. And finally I hope I find time to integrate it till the end of this year into master, so that it is available via pypi.

Greetings Luiko

lczub commented 9 years ago

pre-release with proxy support (but not py3) is published , see v0.5.3-dev-proxy

lczub commented 9 years ago

Hello Mario,

FYI, with 3b8fae8 your ProxiedTransport should now be usable under py3

regards, Luiko

Maberi commented 9 years ago

Thank you, Luiko!

On Sun, Jan 4, 2015 at 6:13 PM, Luiko Czub notifications@github.com wrote:

Hello Mario,

FYI, with 3b8fae8 https://github.com/lczub/TestLink-API-Python-client/commit/3b8fae883fa382ca23a68de644e5c8fa526a5dcb your ProxiedTransport should now be usable under py3

regards, Luiko

— Reply to this email directly or view it on GitHub https://github.com/lczub/TestLink-API-Python-client/pull/36#issuecomment-68640346 .

hellboy81 commented 4 years ago

I am trying to use proxy setting in constructor:

tls = testlink.TestLinkHelper(
    TESTLINK_API_PYTHON_SERVER_URL,
    TESTLINK_API_PYTHON_DEVKEY,
    TESTLINK_API_PYTHON_PROXY,
).connect(testlink.TestlinkAPIClient)

projectsCount = tls.countProjects()

Got exception:

Traceback (most recent call last):
  File "C:/../TestLink-API-Python-client-test/test.py", line 15, in <module>
    tls.countProjects()
  File "C:\...\TestLink-API-Python-client-test\venv\lib\site-packages\testlink\testlinkapi.py", line 392, in countProjects
    projects=self.getProjects()
  File "C:\...\TestLink-API-Python-client-test\venv\lib\site-packages\testlink\testlinkdecorators.py", line 140, in wrapperReplaceTLResponseError
    response = methodAPI(self, *argsPositional, **argsOptional)
  File "C:\...\TestLink-API-Python-client-test\venv\lib\site-packages\testlink\testlinkdecorators.py", line 112, in wrapperAddDevKey
    return methodAPI(self, *argsPositional, **argsOptional)
  File "C:\...\TestLink-API-Python-client-test\venv\lib\site-packages\testlink\testlinkdecorators.py", line 99, in wrapperWithArgs
    return self.callServerWithPosArgs(methodAPI.__name__, 
  File "C:\...\TestLink-API-Python-client-test\venv\lib\site-packages\testlink\testlinkapigeneric.py", line 1582, in callServerWithPosArgs
    response = self._callServer(methodNameAPI, argsOptional)
  File "C:\...\TestLink-API-Python-client-test\venv\lib\site-packages\testlink\testlinkapigeneric.py", line 2057, in _callServer
    response = getattr(self.server.tl, methodNameAPI)(argsAPI)
  File "C:\Python\lib\xmlrpc\client.py", line 1109, in __call__
    return self.__send(self.__name, args)
  File "C:\Python\lib\xmlrpc\client.py", line 1450, in __request
    response = self.__transport.request(
  File "C:\Python\lib\xmlrpc\client.py", line 1153, in request
    return self.single_request(host, handler, request_body, verbose)
  File "C:\Python\lib\xmlrpc\client.py", line 1165, in single_request
    http_conn = self.send_request(host, handler, request_body, verbose)
TypeError: send_request() takes 4 positional arguments but 5 were given
lczub commented 4 years ago

Hello hellboy881,

I guess your report is a duplicate of #127. Try the suggested workaround https://github.com/lczub/TestLink-API-Python-client/issues/127#issuecomment-613473372. Please add further comments on #127.

I will try to find a solution during the next weeks

regards Luiko