Fixed XMPP Support & SleekXMPP dropped#485
Conversation
This adapter had been copied from the SleekXMPP one without ever being tested, no released version of slixmpp ever used the reattempt=False or block=True parameters for these two functions. Additionally, ClientXMPP.connect() only sets up the connection, the actual connection will be established in the asyncio event loop.
Those would require a very specialised XMPP service, if the user has such a server they will be able to modify apprise to support it.
This was the cause of a disconnection in the syntax with non-self JIDs as targets.
This sends it in the message as <subject/>, as defined in RFC 6121.
The previous code was overriding the hostname and the port in all cases, which would basically require the user to do the SRV resolving manually in case their server isn’t on the default port on the same hostname as their JID’s domainpart.
SleekXMPP has been unmaintained for years now, and is unlikely to ever be maintained again given its fork slixmpp brought proper Python 3 support alongside asyncio. In addition, this adapter was already not working for quite a while. It’s time to say good bye. :)
|
Great work! ❤️ I'm still torn with the thought of dropping SleekXMPP; but i guess if it wasn't working for the users of it anyway, it's not a bad idea. The intent of the module in the past was to use Slixmpp if found (in the event they were both present) and fall back to SleekXMPP. I think the last version of SleekXMPP was a bit broken, but if you were to install 1 version back, it worked okay (can't confirm though) I was just curious on your opinion here or thought process behind the removal of the legacy module (for those using legacy versions of Python)? |
|
SleekXMPP hasn’t seen a single commit in three years, last time I’ve heard it was broken with pyasn1 and no one ever stepped up to fix it, I consider it dead. If people want to continue to use that old library, they could also pick an old version of apprise where maybe it would work, but I am in no capacity to maintain or even test it. The only reason I removed it here was that it was preventing some tests from passing. ^^' |
|
@linkmauve I'm okay with your responses; thanks again for such awesome work! |
| # By default we send ourselves a message | ||
| if targets: | ||
| self.targets = parse_list(targets) | ||
| self.targets[0] = self.targets[0][1:] |
There was a problem hiding this comment.
@linkmauve : Your code is already merged, but i just stumbled on this now. I'm just curious if you could explain to me why we're removing the first entry from the list of targets and keeping the rest?
There was a problem hiding this comment.
This is actually the first character of the first entry, which was previously always a '/' and would make the XMPP server close our connection. I’m definitely not sure this was the proper fix, but something somewhere in the parsing of the URI is keeping the initial slash, that is xmpp://foo:bar@baz/quux@target would otherwise try to send a message to='/quux@target', which is an obviously invalid JID and the server rightfully closes our connection. Other targets work fine.
There was a problem hiding this comment.
Thanks for the reply; I believe I tracked the issue down. As a result i created one more small PR (#487) that should fix it and allow you to delimit as many targets as you want on the URL path (/target1/target2/targetN/) . 👍 .
Would you be able to give it a quick test for me? You're so enriched in this XMPP world where as i don't even have a test client 🙂
You should just be able to check the branch out and use the /bin/apprise reference from within it (that will allow you to run an apprise instance against the branch you're within)
3 participants
Description:
Related issue (if applicable): #455
Hi, slixmpp dev here. I fixed a bunch of things in the XMPP plugin, and things work here now. I have fixed the most glaring issues as well, each commit is to be reviewed separately for more context.
Fixes #455.
Checklist
flake8)