smurfix / asynctelnet

Telnet client/server code based on anyio
Other
7 stars 2 forks source link

anyio 3.0 #4

Open uSpike opened 3 years ago

uSpike commented 3 years ago

Hi, just wondering if there's been any work done to port to anyio 3? It's not released yet, but changes could be staged. If not, I'd be glad to contribute. Cheers.

smurfix commented 3 years ago

anyio 3 is (mostly) upward compatible with anyio 2, so "not yet". As of now I'm primarily looking at the test suite. It requires major work. :-/

uSpike commented 3 years ago

OK, then I will submit a PR soon for anyio 3 support. I'm working on a project that uses anyio and asynctelnet, and some anyio 3 features are critical so it'd be great if this could be updated ASAP.

smurfix commented 3 years ago

Cool. Thanks for contributing.

uSpike commented 3 years ago

Oof you're not kidding, the test suite doesn't work hardly at all.

smurfix commented 3 years ago

One test now works. 998 to go …

smurfix commented 3 years ago

Tests for charset and ttype options now work. Needed some nontrivial surgery to clean the stuff up, I was being a tad too optimistic when I first published this :-/

I'll work on merging your PR next, then I'll tackle the remaining heap of testcases.

smurfix commented 3 years ago

Merge done, thanks. I have simplified testcase handling somewhat. If you'd like to contribute fixing some of the remaining tests we should coordinate who'll hack which. Otherwise I'll probably get through them by the end of next week or so.

uSpike commented 3 years ago

Thanks much! I probably wont have time this week, but if you're not done by the weekend I can help out.

smurfix commented 3 years ago

I have ported a few more tests.

The problem that's increasingly apparent with the current structure of the code (and the telnetlib3 code it is based on) is that the whole thing is excessively non-modular and non-extensible. We have three huge classes that contain handlers for a couple of selected Telnet options but no way to add more, or to selectively control which options should be enabled, or which options depend on another (e.g. the default is to require successful TTYPE negotiation before continuing with a bunch of other options, but what if you don't want that for whatever reason but still require charset and echo?), and so on.

Also, basic option handling is (almost) inherently symmetric, DO/DONT on one side and WILL/WONT on the other, and I positively hate to write every piece of code twice. Plus tests. Then there's corner cases like "we're in the middle of CHARSET dance but the remote side has had enough and sends a WONT CHARSET, please clean up" and "I want to wait for successful option-plus-subnegotiation before proceeding" which the current code is not remotely equipped to handle without ugly workarounds.

Thus I'll have to take some time out to think of a more object-oriented way of handling this mess which fixes most, if not all (I hope …) of these issues, without too much special-case code.

There's a lot of random options which don't work in the state the code is in right now, unfortunately. Sorry about that but the end result will hopefully end up a lot cleaner than what we have now. I think the changes so far gave you a good idea how to temporarily work around the current code's problems; I'll try to make the new code's API not too gratuitiously different from the current way of handling things.

uSpike commented 3 years ago

Thanks for the update! My use of this library thus far is extremely lightweight and I'd welcome major API changes. Obviously I can't speak for everyone though.

uSpike commented 3 years ago

I'll avoid opening another issue but FYI it looks like you may have forgotten to add asynctelnet/options.py with https://github.com/smurfix/asynctelnet/commit/7eb76465dc0507d0ffd1ad29a1860af1fc16759b

smurfix commented 3 years ago

Oops, bad checkin. Will fix.

uSpike commented 3 years ago

I'm trying to update to anyio 3.0 but ran into the above comment. I filed another issue for it: #6

dcmeglio commented 1 year ago

Was this effort abandoned?

smurfix commented 1 year ago

Sorry, I ran into a lack of time and had to postpone the project I needed asynctelnet for. It's on my list of stuff to resurrect. Meanwhile patches are welcome.