Skip to content
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

*Pool (AcceptorPool, ListenerPool, etc.) to be overridable? #1323

Open
alexey-pelykh opened this issue Mar 20, 2023 · 10 comments
Open

*Pool (AcceptorPool, ListenerPool, etc.) to be overridable? #1323

alexey-pelykh opened this issue Mar 20, 2023 · 10 comments
Assignees

Comments

@alexey-pelykh
Copy link
Contributor

@abhinavsingh any objections or wishlist items for a PR to make *Pool classes overridable via corresponding flags?

@abhinavsingh
Copy link
Owner

@alexey-pelykh Interesting :) I don't see bigger issue with such a provision (flag based pool class overrides), unless it doesn't become a burden on the library to keep the contract with external pool handlers.

Can you tell me a bit more about why you need a custom pool manager. Also, how different will it be compared to existing pool classes.

@alexey-pelykh
Copy link
Contributor Author

Thus far I have a specific issue with ListenerPool and the listeners themselves - I need to listen on several IP addresses and ports rather than same IP address and multiple ports. Technically, I could've just copied the Proxy class entirely and adjust it there (as there are also some parts that I have to workaround like signals registration) - yet I haven't talked myself into that yet :)

@abhinavsingh
Copy link
Owner

Currently, proxy.py can listen on multiple port for you. With that in mind, I think making some changes can also enable listening on multiple interface/ip along with multiple ports (a.k.a. listeners across a matrix of ip/port combinations). By doing so we will actually not need ability to override listener pools. I also see such a facility can exist natively within proxy.py, rather than a separate pool implementation. Wdyt?

PS: I'll dig back into some of the code and try to pinpoint where changes can be made to enable this capability.

@abhinavsingh
Copy link
Owner

@alexey-pelykh To achieve listening on multiple interfaces(ip) and port matrix, we simply need to override this line https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/core/listener/tcp.py#L76

This line currently directly uses self.flags.hostname which can be removed in favour of an ip_address or interface constructor parameter. Then while initialising ListenerPool, we can pass each combination out of ip-port matrix and initialise all necessary listeners. Acceptor pool is already capable of listening over multiple socks, and they will continue to do their work as is.

Let me know what you think of it and iff such a thing will suffice for your use case.

@abhinavsingh
Copy link
Owner

For a proof-of-concept, if you like, just hardcode your logic on this line to get started https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/core/listener/tcp.py#L76

Instead of str(self.flags.hostname), start using random.choice([ip1, ip2, ip3]) and see if proxy.py keeps working as is without any additional changes. If it does, yay, we have a path forward :)

@alexey-pelykh
Copy link
Contributor Author

@abhinavsingh please see #1325

@alexey-pelykh
Copy link
Contributor Author

alexey-pelykh commented Mar 29, 2023

@abhinavsingh With that PR almost taken care of I still feel there's a bit of extensibility lacking - e.g. some parts of Proxy class are making it bad for my usecase (which is actually not using via command-line but as part of bigger codebase) yet I can not disable or override them, as the code is quite "in the single method". Probably when #1325 is merged, I'll still try to rework #1324 a bit to make everything a bit more overridable.

@abhinavsingh
Copy link
Owner

@alexey-pelykh I am just wondering, whether you are trying to use Proxy within an asyncio context and likely you want the loop to be shared? Just speculating to get some insights. If yes, then may-be, we could solve it natively within Proxy.

I agree somewhat on the "in the single method" issue, I never got to beautify/refactor/generic certain aspects. I envisioned cutting the core into a separate pip package to keep the concerns cleaner, but thats a far fetched thought for me right now.

Overall, I don't see an issue adding pool overrides. However, my first approach would be to add the facilities or carve out APIs natively to allow external bigger projects to seamlessly integrate with Proxy. Wdyt?

@alexey-pelykh
Copy link
Contributor Author

@abhinavsingh in my case it's a relay: a proxy with a per-connection configured upstream proxy. Thus https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/plugin/proxy_pool.py#L72 this specific line being there instead of being lazy-evaluated since the plugin instance is per-connection anyways is an issue for me. Then this line https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/proxy.py#L268 unconditionally hijacks the signals of entire program that I can thankfully workaround by overriding the _register_signals. Architecture-wise I kind-a get it that the structure evolved around the CLI use-case rather than library use-case and there's nothing wrong with it for sure - and since I'm trying to use CLI-favored tool in a library way, obviously that would cause issues :)

@alexey-pelykh
Copy link
Contributor Author

Oh, and rationale behind having custom pools - e.g. I'd prefer to have control over which TcpListeners are created since having "all ports on all addresses" is not good for my case yet it makes perfect sense as evolution of current implementation for the 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

No branches or pull requests

2 participants