Bug28692 #316
Bug28692 #316
Conversation
to a function. Replace comment by docstring. Add an integration test to check the option is set.
The log message might not correspond to the reason. Only the first one that fails will be logged, but not refactoring here. Add an integration test to check a runtime option fails.
and create function to set options that can fail because they are not supported by some Tor versions at runtime. Fixes bug 28692. Bugfix v0.4.0
sbws/util/stem.py
Outdated
| """Set torrc options at runtime.""" | ||
| try: | ||
| cont.set_conf('__DisablePredictedCircuits', '1') | ||
| cont.set_conf('__LeaveStreamsUnattached', '1') |
There was a problem hiding this comment.
Can you refactor these options into a dictionary constant?
And let's put all the options in the same place.
You could put the new constant in globals.py after TORRC_STARTING_POINT.
There was a problem hiding this comment.
That was my initial idea (https://trac.torproject.org/projects/tor/ticket/28692#comment:8):
> Even if only stem.py uses these options, i think they should be in globals.py, since we might need to change them and globals.py should be the place where to change sbws defaults.
Let's leave any refactoring until #28737.
sbws/util/stem.py
Outdated
| @@ -16,6 +18,9 @@ | |||
| stream_building_lock = RLock() | |||
|
|
|||
|
|
|||
| TORRC_OPTIONS_CAN_FAIL = OrderedDict({'ConnectionPadding': '0'}) | |||
There was a problem hiding this comment.
Let's put all the options in the same place.
You could put this constant in globals.py after TORRC_STARTING_POINT.
There was a problem hiding this comment.
That was my initial idea (https://trac.torproject.org/projects/tor/ticket/28692#comment:8)
sbws/util/stem.py
Outdated
| @@ -117,6 +122,33 @@ def _init_controller_socket(socket): | |||
| return c | |||
|
|
|||
|
|
|||
| def set_torrc_runtime_options(cont): | |||
There was a problem hiding this comment.
Please call this function on line 74 as well, to remove duplicate code.
| They can be set at launch, but since the may fail because they are not | ||
| supported in some Tor versions, it's easier to try one by one at runtime | ||
| and ignore the ones that fail. | ||
|
|
| @@ -72,8 +73,7 @@ def init_controller(port=None, path=None, set_custom_stream_settings=True): | |||
| return None, 'Unable to reach tor on control socket' | |||
| assert c is not None | |||
| if set_custom_stream_settings: | |||
| c.set_conf('__DisablePredictedCircuits', '1') | |||
| c.set_conf('__LeaveStreamsUnattached', '1') | |||
| set_torrc_runtime_options(c) | |||
There was a problem hiding this comment.
Do we also need to call set_torrc_options_can_fail(c) here?
| @@ -26,6 +29,13 @@ | |||
| 'ProtocolWarnings': '1', | |||
| } | |||
|
|
|||
| TORRC_RUNTIME_OPTIONS = { | |||
There was a problem hiding this comment.
Do we need a comment here that explains how we use these options?
| '__LeaveStreamsUnattached': '1', | ||
| } | ||
|
|
||
| TORRC_OPTIONS_CAN_FAIL = OrderedDict({'ConnectionPadding': '0'}) |
There was a problem hiding this comment.
Do we need a comment here that explains how we use these options?
|
squashed and rebased in https://github.com/juga0/sbws/commits/bug28692_squashed_rebased. |
|
Merged the squashed branch. |
No description provided.