Code review comment for lp:~gz/bzr/https_proxy_host_matching_944696

Revision history for this message
Vincent Ladeuil (vila) wrote :

As far as testing goes, we already have a case where we need to setup an external test server (bug #866111) and there is also the bzr-local-test-server which could probably do with an added config for tinyproxy.

The change looks fine but I wonder if it will address the (unorthodox but still valid) use case of an https proxy used to talk to a real https server. In other words, relying on self.proxied_host being None may be brittle and may be 'host' should just be a method parameter and let callers set it knowingly.

No need to block on that but doing a quick check or filing a bug for the https -> https case may be good.

« Back to merge proposal