Opened 7 years ago
Closed 5 years ago
#3724 closed enhancement (wontfix)
Support CONF_CHANGED events
Reported by: | krkhan | Owned by: | krkhan |
---|---|---|---|
Priority: | Medium | Milestone: | |
Component: | Torctl | Version: | |
Severity: | Keywords: | ||
Cc: | Actual Points: | ||
Parent ID: | #1692 | Points: | |
Reviewer: | Sponsor: |
Description
#1692 implemented CONF_CHANGED events for Tor which get controllers can use to get notified of any configuration changes made by SETCONF/HUP. TorCtl should support CONF_CHANGED events for its listeners.
Child Tickets
Change History (9)
comment:1 Changed 7 years ago by
Status: | new → needs_review |
---|
comment:2 follow-up: 3 Changed 7 years ago by
comment:3 Changed 7 years ago by
Replying to robinson:
Where is the specification for CONF_CHANGED events? I don't see it in control-spec.txt.
CONF_CHANGED events were committed to the git repo (patch from #1697). Section 4.1.17 of control-spec.txt explain the event.
Would this be the first multi-line event handled by TorCtl? Otherwise, why the additions to _handle1()? Why should multi-line events be handled this way (e.g. set(codes)), rather than each line separately?
Yes, it is indeed the first multi-line event handled by TorCtl. set(codes) merely ensures that all lines belonging to the multi-line event have the same code. Individual lines are parsed after that.
I recommend this be implemented under Daemon's rewrite of the event handler in ticket #3679.
Damian shall be able to make a better judgment on whether multi-line event parsing should be included in #3679's fix.
comment:4 Changed 7 years ago by
Damian shall be able to make a better judgment on whether multi-line event parsing should be included in #3679's fix.
On first glance the patch looks great and is fairly tangential to 3679 since it doesn't follow the positional/keyword argument pattern of most event types. It doesn't conflict and should be trivial to merge.
comment:5 Changed 6 years ago by
Milestone: | Tor: 0.2.3.x-final |
---|
comment:6 Changed 6 years ago by
Little sad to see the milestone removed but makes sense.
This is what prevents controllers from doing client side caching when there are multiple controllers attached. I could partly work around this if we had an event or GETINFO option to figure out if there are other controllers so caching could be disabled in that use case. This is easy to figure out with netstat but we're kinda breaking that. ;)
comment:9 Changed 5 years ago by
Resolution: | → wontfix |
---|---|
Status: | needs_review → closed |
Not vital for TorFlow. Resolving.
https://blog.torproject.org/blog/torctl-deprecation-and-stem-plans
I reviewed this and I have many questions.
Where is the specification for CONF_CHANGED events? I don't see it in control-spec.txt.
Would this be the first multi-line event handled by TorCtl? Otherwise, why the additions to _handle1()? Why should multi-line events be handled this way (e.g. set(codes)), rather than each line separately?
I recommend this be implemented under Daemon's rewrite of the event handler in ticket #3679.