[tor-dev] DetecTor Review
atagar at torproject.org
Sat Jun 7 23:06:07 UTC 2014
Hi Kai. I just took a quick peek at your DetecTor project
(http://detector.io/). It looks like the project has been inactive for
a while so just some quick thoughts in case it sparks back up.
> log_setting = "notice file " + data_dir + os.sep + "tor.log"
The following might be nicer...
log_setting = "notice file %s%stor.log" % (data_dir, os.sep)
Same goes for most instances of string concatenation, actually.
> 'StrictExitNodes': str(1),
Interesting, never seen this before. Should be the following instead.
> def stop_tor(control_port):
> tor_pid = stem.util.system.get_pid_by_port(control_port)
> if tor_pid:
> os.kill(tor_pid, signal.SIGTERM)
Certainly a fine approach. Would you like to avoid having Tor outlive
this python process? If so then that's as easy as including
'take_ownership = True' in your launch_tor_with_config() call. If not
then you can use the process object returned by
launch_tor_with_config() to kill it as well...
tor_process = stem.process.launch_tor_with_config()
... do stuff...
Also this method has tabs mixed with spaces. I would highly recommend
strictly using spaces - python is whitespace dependent, and occasional
tabs can make things a lot more confusing.
> def is_tor_running(control_port):
> return bool(stem.util.system.get_pid_by_port(control_port))
I believe you can use the poll() method of the process object to make
this a little less platform dependent.
> if os.path.isdir(dir_name) != True:
You never need to compare with booleans in an if statement. This is
if not os.path.isdir(dir_name):
Minor thing but this doesn't properly close the file handle. It's
probably done on its own during GC, but it would be more proper to
with open('sphere_control.config') as config_file:
Honestly I'm really not sure what the following code is doing. I
suspect there's a lot of low hanging fruit for improving this (for
instance using "', '.join(countries_list[i])" instead of tracking a
'countries_string') but first step would be some comments explaining
what this is trying to achieve.
More information about the tor-dev