Add a web UI to Cardinal

[johnmaguire]

This is something I experimented with a long time ago. However, I wasn't quite sure how to do threading. I could implement this now.

[s-ol]

the only problem I have right now is that closing the connection is an asynchronous operation in twisted and so Cardinal needs to wait for that Deferred's callback before attempting to re-instantiate the plugin (see the web-plugin branch on my fork)

[johnmaguire]

master...S0lll0s:web-plugin#diff-4e0d2d6392603e098942bdded44b0f30R460 -> Is there a reason you moved the unload up? It was after the try/except block in order to prevent unloading a plugin if there was a syntax error in the updated version, for example.

I'm not quite sure I'm following the issue: Reloading doesn't work, or you can't get it to load up even the first time? Is that also what the referenced change was attempting to alleviate?

Theoretically, could we create a thread from the WebPlugin in order to run the server from there, and have it maintain its own event loop? That's sort of what I was getting at with some of my other comments I guess: Threading isn't built-in to Cardinal, because if a plugin is sure it can be threaded, it should be able to do so itself.

Granted, message passing could be tricky.

[s-ol]

Well, the web plugin ties into the same twisted event loop as cardinal itself (which is what twisted is all about, right?). The reason I moved the unloading block further up was a half-finished attempt at fixing the problem I tried to explain above, let me try again:

The web plugin, like some other future plugins might, uses a resource that can only be owned by one plugin at a time: the webserver port. Because the port is locked in the constructor, it needs to be cleaned up before the next version is instantiated.
My change would be the solution, except that twisted's TCPServer.stop_listening() is asynchronous, so the reloading would need to take that into account which would mean that the close() api needs to be changed (completely?).

There are other workarounds like deferring the server creation until the port is free again (that would bloat the web plugin slightly) but that's where I stopped today and came here for feedback instead because I think this is a relatively deep-running issue that you might want to resolve with a bigger restructuring (like making Cardinal use twisted.internet.defer).

[johnmaguire]

@S0lll0s I will be honest -- I made a scrub move when I initially implemented TwistedIRC into Cardinal. I'm not terribly familiar with Twisted, or its architecture (I know, right?) In fact, I was planning to try to move away from Twisted in the somewhat near future, towards probably: https://github.com/LK-/lurklib

Reasons being:

• Twisted is bloated
• TwistedIRC is fairly undocumented
• I'm familiar with lurklib
• Python 3 compatibility

#47 kind of alludes to this.

For that reason, I'm not terribly tied down to the idea of using Twisted for this. It's certainly an option, and if you think it's a good call, I'm down. But I think I'm going to need to start really understanding how Twisted is meant to be used, which means I don't have a great answer for you right this second. :(

[johnmaguire]

Okay, so I did some brief reading on twisted.internet.defer (I will be reading up more on Twisted, don't worry), and from what it sounds like, I'd expect TCPServer.stop_listening returns a Deferred object. In that case, we could have close() optionally return a Deferred, and if one is returned, wait to load the new plugin until it finishes successfully (or possibly even try if it errors.)

Disadvantages in my mind are:

1. What if it never executes? (Probably not an issue with TCPServer.stop_listening() but could be an issue with plugins that are written improperly) Can we maybe have a timeout and try to force a reload anyway? Does that even make sense to do?
2. Wouldn't necessarily be able to return the entire reload output in one go. If the Deferred takes a long time to execute, it would suck to block Cardinal on .reload. This means some more core changes to how Cardinal handles async (not necessarily a bad thing.) If we go that route, it may make sense to implement Deferred handling in commands as well.

I hope I'm understanding correctly.

[johnmaguire]

I suppose another method might be to shift the responsibility to the plugin -- I'm thinking essentially that the plugin would define a callback to be added as a callback to the Deferred returned by stop_listening() that would cause it to load again. You would then either raise a message or give some sort of return that would indicate to Cardinal not to load that plugin itself. This is a lot nastier I think though -- you'd have to let the plugin know whether it's meant to be reloaded or not, and if you ran .unload and then .load manually, you hit the race condition anyway.

[s-ol]

Exactly my thoughts basically (apart from the second option, I don't think it will work well and it would be rather un-DRY). Timeouts shouldn't be hard to do I think, deferreds can be failed too and the handler would still work, but some changes to Cardinal's core would definetely need to be made to ensure that commands don't completely block each other I think.

[johnmaguire]

Yeah, the second option seemed pretty nasty. :P I try to consider at least two options before going with one. I am going to try to take a look at her command handling later today or tomorrow and try to determine a good way to implement async across her entirety. I'm also making my way through this introduction to Twisted.

[s-ol]

I don't really know twisted either so I guess I should read that too....

[johnmaguire]

It's long. :P I don't know how necessary it is to read the whole thing, but I figure if I've been working on a Twisted-based project for a couple years, I should probably figure out what the actual core library is supposed to do. :P

[s-ol]

What facilities for plugin-to-plugin communication are there currently, except custom event types? How would web-plugins make themselves known to the web plugin? What about load oder (and reload oder!)?

[johnmaguire]

@S0lll0s That's a fun one! And another thing I've been thinking about recently.

Hah! It looks like I closed out #9 where I first documented my thoughts on how plugin inter-op could work two years ago, because #17 encompassed it, but then I ignored it when implementing #17.

It should be a pretty simple addition. I think the API would have to be an object registered to the PluginManager in some way by the plugin object. Possibly even a def api() similar to def setup() that accepts something. I'm not really sure, any ideas?

As for load order, I was thinking about that for #79 actually. Some sort of dependencies field will be necessary pretty soon. I need to sleep on it.

[s-ol]

API like in #9 looks like it would work fine, I imagine the load order thing could work if you had two loading stages, first all plugins are instantiated, then they are start()ed. That way all APIs are already loaded. That still doesn't cover reloading (because what happens if the web plugin gets reloaded but a plugin hooking into it doesn't) but that could be handled with events.

This would make plugin dev pretty complicated though, a dependency system would solve most issues in the main codebase already.