Skip to content

Proxy configuration support in TestLinkHelper #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

Maberi
Copy link

@Maberi Maberi commented Nov 25, 2014

This patch allows easy proxy configuration using TestLinkHelper.

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

@lczub
Copy link
Owner

lczub commented Nov 25, 2014

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
Copy link
Author

Maberi commented Nov 25, 2014

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
Copy link
Owner

lczub commented Nov 26, 2014

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 pushed a commit that referenced this pull request Dec 8, 2014
new class ProciedTransport extraced in separate file
lczub pushed a commit that referenced this pull request Dec 8, 2014
pull request #36 does not use default settings from environment for
arguments, when they are not set via command line 

ERROR: test_setParamsFromArgs
(testlinkhelpertest.TestLinkHelperTestCase)
set TestLinkHelper params from command line arguments
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"D:\Projekte\git\TestLink-API-Python-client\test\utest-offline\testlinkhelpertest.py",
line 125, in test_setParamsFromArgs
    '--devKey' , 'DEVKEY-41'])
  File
"D:\Projekte\git\TestLink-API-Python-client\src\testlink\testlinkhelper.py",
line 194, in setParamsFromArgs
    self._server_url = args.server_url
AttributeError: 'list' object has no attribute 'server_url'
lczub pushed a commit that referenced this pull request Dec 8, 2014
lczub pushed a commit that referenced this pull request Dec 10, 2014
@lczub
Copy link
Owner

lczub commented Dec 11, 2014

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

@lczub
Copy link
Owner

lczub commented Jan 4, 2015

Hello Mario,

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

regards, Luiko

@Maberi
Copy link
Author

Maberi commented Jan 5, 2015

Thank you, Luiko!

On Sun, Jan 4, 2015 at 6:13 PM, Luiko Czub [email protected] wrote:

Hello Mario,

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

regards, Luiko


Reply to this email directly or view it on GitHub
#36 (comment)
.

@hellboy81
Copy link

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
Copy link
Owner

lczub commented May 5, 2020

Hello hellboy881,

I guess your report is a duplicate of #127.
Try the suggested workaround #127 (comment).
Please add further comments on #127.

I will try to find a solution during the next weeks

regards Luiko

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