Opened 7 years ago

Closed 7 years ago

#7742 closed enhancement (wontfix)

re-attach event listeners -- reloaded

Reported by: robinson Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords: events
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Damian,

I looked more closely at the various authentication paths after your email[0]. And I concluded, I still don't like the hook in BaseController.msg. 8-)

This patch set[1] is my second attempt to find an alternative approach. Basically, call _post_authentication() at the end of each connection.authentication* function. This covers 2 of the 3 use cases you presented.

If someone goes to the trouble to hand-compose an AUTHENTICATE message string to send through BC.msg, they should be sophisticated enough to re-attach event listeners on their own. Someone reaching that deeply into the abstraction layer can protect themselves.

This is my last, best argument for moving the hook. I respect your decision either way.

[0]: https://lists.torproject.org/pipermail/tor-dev/2012-December/004276.html
[1]: https://gitorious.org/stem-robinson/stem-robinson/commits/exp-reattach-listeners-v2

Child Tickets

Change History (3)

comment:1 Changed 7 years ago by atagar

Hi Sean. I've been staring at this ticket for fifteen minutes but I still can't bring myself to think of either solution as being especially elegant. I definitely like the testing addition (thanks!), but replacing the msg() hook with four authenticate_* calls doesn't feel like an improvement.

I'm gonna keep with the msg() hook for now - sorry!

PS. Ravi, Eoin, others: if anyone else wants to take a position on this then I'm fine with merging if I'm outvoted (I'll leave this around for a little while to see).

comment:2 in reply to:  1 Changed 7 years ago by robinson

Replying to atagar:

I'm gonna keep with the msg() hook for now - sorry!

No worries. I appreciate that you spent time looking at my alternatives.

Feel free to close this ticket when you are satisfied no one else objects.

comment:3 Changed 7 years ago by atagar

Resolution: wontfix
Status: newclosed

Feel free to close this ticket when you are satisfied no one else objects.

Making it so. Feel free to reopen this if anyone wants to discuss it further.

Note: See TracTickets for help on using tickets.