Merge lp:~alecu/ubuntuone-client/proxy-tunnel-server into lp:ubuntuone-client
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Natalia Bidart on 2012-03-08 | ||||
| Approved revision: | 1211 | ||||
| Merged at revision: | 1202 | ||||
| Proposed branch: | lp:~alecu/ubuntuone-client/proxy-tunnel-server | ||||
| Merge into: | lp:ubuntuone-client | ||||
| Diff against target: |
953 lines (+899/-1) 9 files modified
Makefile.am (+2/-1) tests/proxy/__init__.py (+148/-0) tests/proxy/ssl/dummy.cert (+19/-0) tests/proxy/ssl/dummy.key (+16/-0) tests/proxy/test_tunnel_server.py (+351/-0) ubuntuone/proxy/__init__.py (+14/-0) ubuntuone/proxy/common.py (+59/-0) ubuntuone/proxy/logger.py (+37/-0) ubuntuone/proxy/tunnel_server.py (+253/-0) |
||||
| To merge this branch: | bzr merge lp:~alecu/ubuntuone-client/proxy-tunnel-server | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Natalia Bidart | 2012-02-28 | Approve on 2012-03-08 | |
| Manuel de la Peña (community) | 2012-02-28 | Approve on 2012-03-07 | |
|
Review via email:
|
|||
Commit Message
- The infrastructure for a QtNetwork based process to tunnel syncdaemon traffic thru proxies. (LP: #929207)
Description of the Change
- The infrastructure for a QtNetwork based process to tunnel syncdaemon traffic thru proxies. (LP: #929207)
| Alejandro J. Cura (alecu) wrote : | # |
> I dont think that the following code is correct:
>
> 787 + else:
> 788 + return QNetworkProxy(
> 789 + hostName=
> 790 + port=settings.
> 791 + user=settings.
> 792 + password=
>
> The use of "" for the username and password will give problems when dealing
> with the auth proxies because the first attempt would be to use them and
> therefore you will get a 401 instead of a 407.
Those values are only used when the "settings" dictionary has no key with the given name.
And I'm using empty strings because that's what QNetworkProxy uses if no parameter is given:
* https:/
Since the current version of the tunnel is not getting the authentication, I think it's safe to use it like this. Perhaps we can add a comment or a TODO for it, and we can fix it in an upcoming branch to add authentication.
> 506 + def test_stop_
> 507 + """Stop but it was never connected."""
> 508 + fake_protocol = FakeServerTunne
> 509 + client = tunnel_
> 510 + yield client.stop()
>
> There is a missing decorator in the test.
Great catch! Thanks, I'm fixing it too.
- 1208. By Alejandro J. Cura on 2012-03-01
-
fixes requested on the review
- 1209. By Alejandro J. Cura on 2012-03-02
-
Use a QTimer to work around flaky signals
- 1210. By Alejandro J. Cura on 2012-03-06
-
merged with trunk
| Natalia Bidart (nataliabidart) wrote : | # |
* Can't we re-use teh code from ubuntu-sso-client instead of defnining:
+def build_proxy(
?
* Most of our projects and source file use this syntax for file encoding:
# -*- coding: utf8 -*-
would you consider using that form?
* There should be spaces around the % in "HTTP/1.0 %d %s"%(code, description)
The branch looks good!
| Alejandro J. Cura (alecu) wrote : | # |
> * Can't we re-use teh code from ubuntu-sso-client instead of defnining:
>
> +def build_proxy(
>
> ?
The bits of code that build the QNetworkProxy instance and configure the proxy settings in ussoc are currently in flux, because mandel is changing it to use a QNetworkProxyFa
> * Most of our projects and source file use this syntax for file encoding:
>
> # -*- coding: utf8 -*-
>
> would you consider using that form?
Sure, fixing that.
> * There should be spaces around the % in "HTTP/1.0 %d %s"%(code, description)
Fixing that too.
thanks for the review!
- 1211. By Alejandro J. Cura on 2012-03-07
-
fixes requested in review


I dont think that the following code is correct:
787 + else: QNetworkProxy. HttpProxy, settings. get("host" , ""), get("port" , 0), get("username" , ""), settings. get("password" , ""))
788 + return QNetworkProxy(
789 + hostName=
790 + port=settings.
791 + user=settings.
792 + password=
The use of "" for the username and password will give problems when dealing with the auth proxies because the first attempt would be to use them and therefore you will get a 401 instead of a 407.
505 + but_never_ connected( self): lProtocol( ) server. RemoteSocket( fake_protocol)
506 + def test_stop_
507 + """Stop but it was never connected."""
508 + fake_protocol = FakeServerTunne
509 + client = tunnel_
510 + yield client.stop()
There is a missing decorator in the test.