search

Jamie- / openvpn-api

A Python API for the OpenVPN management interface.
MIT License
60 stars 18 forks source link

Event system #14

Open HosseyNJF opened 4 years ago

HosseyNJF commented 4 years ago

There is one event class implemented now, the ClientEvent class is handling events coming from client interaction. I wrote two threads for the connection process with sockets; They're working as expected but probably this PR requires much more testing to be mergeable.

HosseyNJF commented 4 years ago

Any feedback?

Jamie- commented 4 years ago

Thanks for the PR, I'm incredibly busy at the moment on other projects I'm trying to push live so don't have the time yet to review these changes as they're quite big. Sorry it's slow. I'll get to them.

Jamie- commented 4 years ago

Master has since changed so I've created a branch and fixed the conflicts: https://github.com/Jamie-/openvpn-api/tree/event-system

HosseyNJF commented 4 years ago

Thank you :) You've already done most of the reviews; Is there anything that I could add to it or should we merge it? I guess it still needs some real-life and high-stress testing.

Jamie- commented 4 years ago

Have also just discovered sockets are not thread safe, so having two threads (a send and a receive) is a "bad thing". Just about to commit a fix for that which puts all socket communication into one thread.

But yes, as you say, it does need more unittest testing, and some real life testing, and ideally some load testing too. That would be cool

HosseyNJF commented 4 years ago

I discovered that 4096 is a real low buffer size, and the client-connect event never reaches the callback because it gets split up with the low buffer, and an exception gets thrown. maybe we should increase it? It won't even fit a single event, not to mention multiple events coming at the same time.

Edit: seems like OpenVPN actually sends one event in multiple packets, and the bug won't be fixed by just increasing buffer size.

HosseyNJF commented 4 years ago

I fixed this bug and some others in my repo: HosseyNJF/openvpn-api

HosseyNJF commented 3 years ago

I've been using this on a production server for 4 months now, and everything's looking stable, although my fix of the buffer bug isn't waterproof and probably is going to fail sometime. Any updates?

HosseyNJF commented 9 months ago

Hi @Jamie-. I know this is an ancient PR, but it would be a waste to abandon this feature :) If you are up for it. I can start testing the event-system branch present on this repo (not my fork) on a production server, and give feedback so that this feature could be present in the master.