Closed ekimekim closed 10 years ago
I like parts of your initial implementation. However, I really don't think we should be mucking around the core for this. I'd created LocalUser
for basically the exact purpose of modules that need to have a pseudouser around.
I do really like the base Service class, though. Can we implement that in maybe a services base module that uses LocalUser
to implement the user? Maybe we want to do this anyway, because I don't see anywhere in your patch where the service actually becomes a user. You've hacked in that they can receive messages, but there are other things to being a user--for example, the WHOIS command won't work as expected. Also, it's very easy to impersonate services in this system, as there's nothing stopping a user from changing nicks to one a service is using, and now the message is sent to both, and the user gets passwords and stuff for free. Hacking services into the core like this is going to be a disaster for anywhere that refers to users, whereas LocalUser
is already ready to plug into the core. (Looks like I derped, though, and it doesn't add itself like it's supposed to. I'll put it on my list for today.)
Also, since you're no longer in the IRC: <+ekimekim__> it's easier to write them as distributed
Not everything is on the MySQL server--parts of NickServ and BidServ data, as well as all of ChanServ, would need to be properly synced and updated between servers. The especially hard part is that we have to consolidate all this data to be correct and consistent in the case of a netsplit/netmerge. There's a netsplit, and we go on acting like everything is fine, but then we have to merge the data back together and still make it seem like everything was fine (with maybe a couple account collisions as collateral damage). This is a lot easier when we don't have to deal with user data and probably even easier for the Desert Bus case where there's basically no changing of channel data during, and when there is it's sporadic and easy to update, but if we're looking to create general distributed services (which would be nice), we do have to consider those.
Yep, that's pretty much what I expected you'd say. And you're right. I just didn't know how I'd go about creating a local user, and so decided to focus on getting something working instead with what I did know. I fully expected this commit to only be a starting point. Also fair point re: distributed services, and I suppose that if we're configuring our servers in hub-and-spoke (single "central" server all the others connect to) formation, setting up master/slave services is simple enough.
Will reply more in-depth + with code tomorrow, for now there's day jobs to be getting on with :(
Pushed a new implementation. WORK IN PROGRESS. It doesn't work currently. I'm just looking for feedback. Also notice that I'm rebased onto Heufy's looping call fix.
The error handling for dataReceived
turned out to be needed sooner than when this PR will be done. I did it myself forgetting that you'd made a commit about it. I've changed mine to be somewhat more like yours, since yours makes more sense.
Heufy's PR is merged now, as well.
I'll look over the rest later today.
I've made a variety of commit comments. I don't really have any general comments that I didn't already say in the commit comments. Let me know how things go. :)
It's looking better. :)
poke.
Moved service out of core. Fixed all later commits to use the new import.
I've created a basic implementation of services with some built-in functionality. This should let us build the services quickly and easily.