Skip to content

Conversation

@tkluck
Copy link
Owner

@tkluck tkluck commented Feb 3, 2019

This is an attempt at offloading the actual proxying to a high-quality proxy, namely tinyproxy. This hopefully solves issues such as #56 (or at least makes those kinds of issues not our responsibility).

More generally, a C implementation is probably more performant than the Twisted-based proxy. Although performance has never been an issue (I've streamed netflix/youtube over pac4cli), it's a good step towards the general acceptability of pac4cli if we ever want to push for installing by default (e.g. at Booking.com where I work, or more generally in linux distributions in much the same way as dnsmasq is coupled with NetworkManager).

This needs a small modification to tinyproxy to make it link in pacparser and use it. The current code is just a proof-of-concept: it has a memory leak and it should have better semantics for when the user configures both a PAC file and an Upstream. But if the code quality is addressed, it's a reasonable enough change that we may be able to upstream it.

@tkluck
Copy link
Owner Author

tkluck commented Feb 4, 2019

@denilsonsa @kdehairy @lucrocha how would you feel about replacing the proxy part by a subprocess as described above?

@tkluck
Copy link
Owner Author

tkluck commented Feb 4, 2019

Opened tinyproxy/tinyproxy#224 to see if they'd be willing to upstream such a thing.

@denilsonsa
Copy link
Collaborator

how would you feel about replacing the proxy part by a subprocess as described above?

I'm split because...

  • Using the built-in proxy has the advantage of less dependencies and easier to setup/build/get-it-running.
  • Using the external proxy has the potential of being faster and more accurate, but more troublesome to setup for some people. And, to make it worse, we have to use our own fork of the external proxy.

So, part of me thinks the code could be slightly refactored to support both cases, and part of me doesn't want the feature-creep of supporting both solutions.

In summary, if the building and setup of the project is easy enough, I'm in favor of it! Even better if we can pick a proxy server that requires no changes.

@tkluck
Copy link
Owner Author

tkluck commented Feb 11, 2019

@denilsonsa excellent analysis, I agree with it.

If upstream doesn't want to merge these kinds of changes, building&setup of the project will suffer a lot. So it's probably best not to pursue this unless we can find a different project. :(

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