Opened 13 years ago
Last modified 13 years ago
#9603 new defect
Maintainership of the Growl Plugin
Reported by: | Aurélien Bauchet | Owned by: | Emmanuel Blot |
---|---|---|---|
Priority: | normal | Component: | GrowlPlugin |
Severity: | normal | Keywords: | Maintainership |
Cc: | Trac Release: | 0.11 |
Description
Hello everybody,
I would like to know if this plugin is still being maintained by someone at least. I hacked on it and migrated it to the new GNTP protocol which allows to make it work with the newer versions of Growl and Growl for Windows.
I, at least, would like to commit the code but I also don't mind getting the maintainership of this plugin.
Attachments (0)
Change History (11)
comment:1 Changed 13 years ago by
comment:2 follow-up: 3 Changed 13 years ago by
The new protocol is indeed TCP-based, as the growl developers wanted to have a bi-directive communication.
The UDP protocol has been droped at least by Growl(Mac) and Growl for Windows, which I use. Growl for Windows can only receive notifications from the GNTP protocol but can broadcast them in UDP to older clients.
comment:3 Changed 13 years ago by
Replying to azurief:
The new protocol is indeed TCP-based, as the growl developers wanted to have a bi-directive communication.
Very bad idea(TM)
Ok, so a new configuration option is required to support this new, heavy protocol. Deadlocking the Trac webserver to push a notification will a synonym of major potential issues ahead.
Does your patch support this kind of option?
comment:5 follow-up: 6 Changed 13 years ago by
I need to modify the patch to implement a configuration option but my patch does indeed support issues where the clients don't answer to the server sending notifications. If there's a problem sending notifications to one of the clients, it's logged but there's no deadlocking on Trac side.
comment:6 Changed 13 years ago by
Replying to azurief:
I need to modify the patch to implement a configuration option but my patch does indeed support issues where the clients don't answer to the server sending notifications. If there's a problem sending notifications to one of the clients, it's logged but there's no deadlocking on Trac side.
I was referring to the TCP level: you do manage low level TCP connection issues?
comment:7 Changed 13 years ago by
The low level TCP connection is handled by the GNTP python library. This library is referenced on the Growl for Windows website and is actively being maintained.
I know that this plugin was originally a standalone one not relying on any third party library but the new protocol being more complex than the UDP one, I prefer to rely on something used and validated rather than redo the same thing.
comment:8 Changed 13 years ago by
It does not seem that connection issues are not managed at all within this library:
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.connect((self.hostname, self.port)) s.send(data.encode('utf8', 'replace')) response = gntp.parse_gntp(s.recv(1024)) s.close()
Timeout management seems to be the default one, etc. In order words, if the receiving side is accepting the connection and stops, Trac will simply freeze.
It exactly looks the same as the default SMTP notification from Trac, with an even less reliable peer as Growl "server" is a brand new implementation and runs on a less reliable machine than a full blown server most of the time.
I don't mind adding this new protocol to the plugin, as it is a required protocol for the paid version of Growl but:
- the documentation will require a serious disclaimer,
- previous protocol should be kept with a version switch (UDP / GNTP) for those who want to be sure that Trac can run w/o deadlocks.
What a strange idea to enforce TCP for a notification subsystem :-(
comment:9 follow-up: 10 Changed 13 years ago by
Ok, indeed, didn't consider this use case...
I will rework my code at the same time to implement a queue. This way notification will handle asynchronously and won't block Trac.
comment:10 Changed 13 years ago by
Replying to azurief:
I will rework my code at the same time to implement a queue. This way notification will handle asynchronously and won't block Trac.
Hmmm. Do not: managing background threads is even more difficult. This is why it has never been implemented for the SMTP notification. Corner cases are even more difficult to manage (shutdown being one of them) - not mentioning that Trac itself can run from multi-processes and/or multithreads.
I guess the best option is to tweak the TCP connection parameters to reduce deadlocks - if it is even possible.
TCP is evil for this kind of task.
comment:11 Changed 13 years ago by
Ok, I looked out a bit and the following looks interesing : Blocking/Unblocking mode for sockets
This might be the workaround needed. Will check out if the GNTP library make use of it.
Not actively maintained, but if you have a patch for the new protocol, I'll be glad to commit it.
However, how the new protocol works? I read a long time ago that the Growl developers wanted to get rid of the UDP protocol, which is IMHO just a huge mistake.
Is GNTP TCP-based, or is it still possible to use UDP?