[tor-commits] [ooni-probe/master] Fix bug that lead to some reports not being submitted.
art at torproject.org
art at torproject.org
Thu Jun 26 13:58:11 UTC 2014
commit 74a146834ee6609fd4f66c0b5f5381654acf8feb
Author: Arturo Filastò <art at fuffa.org>
Date: Thu Jun 19 16:06:50 2014 +0200
Fix bug that lead to some reports not being submitted.
Pep8 related fixes.
---
ooni/deck.py | 14 ++++---
ooni/managers.py | 22 ++++++----
ooni/nettest.py | 123 +++++++++++++++++++++++++++++++++---------------------
ooni/oonicli.py | 81 +++++++++++++++++++----------------
ooni/reporter.py | 3 +-
5 files changed, 144 insertions(+), 99 deletions(-)
diff --git a/ooni/deck.py b/ooni/deck.py
index ad90d03..f38b098 100644
--- a/ooni/deck.py
+++ b/ooni/deck.py
@@ -7,10 +7,9 @@ from ooni.utils import log
from ooni import errors as e
from twisted.python.filepath import FilePath
-from twisted.internet import reactor, defer
+from twisted.internet import defer
import os
-import re
import yaml
import json
from hashlib import sha256
@@ -187,12 +186,15 @@ class Deck(InputFile):
response = yield self.oonibclient.lookupTestHelpers(required_test_helpers)
for net_test_loader in self.netTestLoaders:
- log.msg("Setting collector and test helpers for %s" % net_test_loader.testDetails['test_name'])
+ log.msg("Setting collector and test helpers for %s" %
+ net_test_loader.testDetails['test_name'])
# Only set the collector if the no collector has been specified
# from the command line or via the test deck.
- if not required_test_helpers and net_test_loader in requires_collector:
- log.msg("Using the default collector: %s" % response['default']['collector'])
+ if not net_test_loader.requiredTestHelpers and \
+ net_test_loader in requires_collector:
+ log.msg("Using the default collector: %s" %
+ response['default']['collector'])
net_test_loader.collector = response['default']['collector'].encode('utf-8')
continue
@@ -223,6 +225,6 @@ class Deck(InputFile):
try:
input_file.verify()
except AssertionError:
- raise e.UnableToLoadDeckInput, cached_path
+ raise e.UnableToLoadDeckInput
i['test_class'].localOptions[i['key']] = input_file.cached_file
diff --git a/ooni/managers.py b/ooni/managers.py
index d03d2c9..65e4ce1 100644
--- a/ooni/managers.py
+++ b/ooni/managers.py
@@ -1,9 +1,9 @@
import itertools
-from twisted.internet import defer
from ooni.utils import log
from ooni.settings import config
+
def makeIterable(item):
"""
Takes as argument or an iterable and if it's not an iterable object then it
@@ -15,6 +15,7 @@ def makeIterable(item):
iterable = iter([item])
return iterable
+
class TaskManager(object):
retries = 2
concurrency = 10
@@ -60,7 +61,7 @@ class TaskManager(object):
self._run(task)
except StopIteration:
break
- except ValueError as exc:
+ except ValueError:
# XXX this is a workaround the race condition that leads the
# _tasks generator to throw the exception
# ValueError: generator already called.
@@ -103,8 +104,8 @@ class TaskManager(object):
def schedule(self, task_or_task_iterator):
"""
- Takes as argument a single task or a task iterable and appends it to the task
- generator queue.
+ Takes as argument a single task or a task iterable and appends it to
+ the task generator queue.
"""
log.debug("Starting this task %s" % repr(task_or_task_iterator))
@@ -136,7 +137,9 @@ class TaskManager(object):
"""
raise NotImplemented
+
class LinkedTaskManager(TaskManager):
+
def __init__(self):
super(LinkedTaskManager, self).__init__()
self.child = None
@@ -160,10 +163,13 @@ class LinkedTaskManager(TaskManager):
if self.parent:
self.parent._fillSlots()
+
class MeasurementManager(LinkedTaskManager):
+
"""
- This is the Measurement Tracker. In here we keep track of active measurements
- and issue new measurements once the active ones have been completed.
+ This is the Measurement Tracker. In here we keep track of active
+ measurements and issue new measurements once the active ones have been
+ completed.
MeasurementTracker does not keep track of the typology of measurements that
it is running. It just considers a measurement something that has an input
@@ -172,6 +178,7 @@ class MeasurementManager(LinkedTaskManager):
NetTest on the contrary is aware of the typology of measurements that it is
dispatching as they are logically grouped by test file.
"""
+
def __init__(self):
if config.advanced.measurement_retries:
self.retries = config.advanced.measurement_retries
@@ -186,7 +193,9 @@ class MeasurementManager(LinkedTaskManager):
def failed(self, failure, measurement):
pass
+
class ReportEntryManager(LinkedTaskManager):
+
def __init__(self):
if config.advanced.reporting_retries:
self.retries = config.advanced.reporting_retries
@@ -200,4 +209,3 @@ class ReportEntryManager(LinkedTaskManager):
def failed(self, failure, task):
pass
-
diff --git a/ooni/nettest.py b/ooni/nettest.py
index 166a4f9..1547617 100644
--- a/ooni/nettest.py
+++ b/ooni/nettest.py
@@ -16,9 +16,11 @@ from ooni import errors as e
from inspect import getmembers
from StringIO import StringIO
+
class NoTestCasesFound(Exception):
pass
+
def get_test_methods(item, method_prefix="test_"):
"""
Look for test_ methods in subclasses of NetTestCase
@@ -36,6 +38,7 @@ def get_test_methods(item, method_prefix="test_"):
pass
return test_cases
+
def getTestClassFromFile(net_test_file):
"""
Will return the first class that is an instance of NetTestCase.
@@ -51,10 +54,12 @@ def getTestClassFromFile(net_test_file):
except (TypeError, AssertionError):
pass
+
def getOption(opt_parameter, required_options, type='text'):
"""
Arguments:
- usage_options: a list as should be the optParameters of an UsageOptions class.
+ usage_options: a list as should be the optParameters of an UsageOptions
+ class.
required_options: a list containing the strings of the options that are
required.
@@ -77,16 +82,19 @@ def getOption(opt_parameter, required_options, type='text'):
required = False
return {'description': description,
- 'value': default, 'required': required,
- 'type': type
- }
+ 'value': default, 'required': required,
+ 'type': type
+ }
+
def getArguments(test_class):
arguments = {}
if test_class.inputFile:
option_name = test_class.inputFile[0]
- arguments[option_name] = getOption(test_class.inputFile,
- test_class.requiredOptions, type='file')
+ arguments[option_name] = getOption(
+ test_class.inputFile,
+ test_class.requiredOptions,
+ type='file')
try:
list(test_class.usageOptions.optParameters)
except AttributeError:
@@ -94,16 +102,20 @@ def getArguments(test_class):
for opt_parameter in test_class.usageOptions.optParameters:
option_name = opt_parameter[0]
- opt_type="text"
+ opt_type = "text"
if opt_parameter[3].lower().startswith("file"):
- opt_type="file"
- arguments[option_name] = getOption(opt_parameter,
- test_class.requiredOptions, type=opt_type)
+ opt_type = "file"
+ arguments[option_name] = getOption(
+ opt_parameter,
+ test_class.requiredOptions,
+ type=opt_type)
return arguments
+
def test_class_name_to_name(test_class_name):
- return test_class_name.lower().replace(' ','_')
+ return test_class_name.lower().replace(' ', '_')
+
def getNetTestInformation(net_test_file):
"""
@@ -122,21 +134,23 @@ def getNetTestInformation(net_test_file):
test_id = os.path.basename(net_test_file).replace('.py', '')
information = {'id': test_id,
- 'name': test_class.name,
- 'description': test_class.description,
- 'version': test_class.version,
- 'arguments': getArguments(test_class),
- 'path': net_test_file,
- }
+ 'name': test_class.name,
+ 'description': test_class.description,
+ 'version': test_class.version,
+ 'arguments': getArguments(test_class),
+ 'path': net_test_file,
+ }
return information
+
class NetTestLoader(object):
method_prefix = 'test'
collector = None
requiresTor = False
def __init__(self, options, test_file=None, test_string=None):
- self.onionInputRegex = re.compile("(httpo://[a-z0-9]{16}\.onion)/input/([a-z0-9]{64})$")
+ self.onionInputRegex = re.compile(
+ "(httpo://[a-z0-9]{16}\.onion)/input/([a-z0-9]{64})$")
self.options = options
self.testCases = []
@@ -204,20 +218,19 @@ class NetTestLoader(object):
input_file_hashes.append(input_file['hash'])
test_details = {'start_time': time.time(),
- 'probe_asn': config.probe_ip.geodata['asn'],
- 'probe_cc': config.probe_ip.geodata['countrycode'],
- 'probe_ip': config.probe_ip.geodata['ip'],
- 'probe_city': config.probe_ip.geodata['city'],
- 'test_name': self.testName,
- 'test_version': self.testVersion,
- 'software_name': 'ooniprobe',
- 'software_version': software_version,
- 'options': self.options,
- 'input_hashes': input_file_hashes
- }
+ 'probe_asn': config.probe_ip.geodata['asn'],
+ 'probe_cc': config.probe_ip.geodata['countrycode'],
+ 'probe_ip': config.probe_ip.geodata['ip'],
+ 'probe_city': config.probe_ip.geodata['city'],
+ 'test_name': self.testName,
+ 'test_version': self.testVersion,
+ 'software_name': 'ooniprobe',
+ 'software_version': software_version,
+ 'options': self.options,
+ 'input_hashes': input_file_hashes
+ }
return test_details
-
def _parseNetTestOptions(self, klass):
"""
Helper method to assemble the options into a single UsageOptions object
@@ -360,7 +373,9 @@ class NetTestLoader(object):
pass
return test_cases
+
class NetTestState(object):
+
def __init__(self, allTasksDone):
"""
This keeps track of the state of a running NetTests case.
@@ -407,6 +422,7 @@ class NetTestState(object):
self.completedScheduling = True
self.checkAllTasksDone()
+
class NetTest(object):
director = None
@@ -480,15 +496,17 @@ class NetTest(object):
if self.director:
measurement.done.addCallback(self.director.measurementSucceeded,
- measurement)
+ measurement)
measurement.done.addErrback(self.director.measurementFailed,
- measurement)
+ measurement)
return measurement
@defer.inlineCallbacks
def initializeInputProcessor(self):
for test_class, _ in self.testCases:
- test_class.inputs = yield defer.maybeDeferred(test_class().getInputProcessor)
+ test_class.inputs = yield defer.maybeDeferred(
+ test_class().getInputProcessor
+ )
if not test_class.inputs:
test_class.inputs = [None]
@@ -506,7 +524,10 @@ class NetTest(object):
test_instance.summary = self.summary
for method in test_methods:
log.debug("Running %s %s" % (test_class, method))
- measurement = self.makeMeasurement(test_instance, method, input)
+ measurement = self.makeMeasurement(
+ test_instance,
+ method,
+ input)
measurements.append(measurement.done)
self.state.taskCreated()
yield measurement
@@ -519,6 +540,7 @@ class NetTest(object):
# Call the postProcessor, which must return a single report
# or a deferred
post.addCallback(test_instance.postProcessor)
+
def noPostProcessor(failure, report):
failure.trap(e.NoPostProcessor)
return report
@@ -526,30 +548,32 @@ class NetTest(object):
post.addCallback(self.report.write)
if self.report and self.director:
- #ghetto hax to keep NetTestState counts are accurate
+ # ghetto hax to keep NetTestState counts are accurate
[post.addBoth(self.doneReport) for _ in measurements]
self.state.allTasksScheduled()
+
class NetTestCase(object):
+
"""
This is the base of the OONI nettest universe. When you write a nettest
you will subclass this object.
* inputs: can be set to a static set of inputs. All the tests (the methods
- starting with the "test" prefix) will be run once per input. At every run
- the _input_ attribute of the TestCase instance will be set to the value of
- the current iteration over inputs. Any python iterable object can be set
- to inputs.
+ starting with the "test" prefix) will be run once per input. At every
+ run the _input_ attribute of the TestCase instance will be set to the
+ value of the current iteration over inputs. Any python iterable object
+ can be set to inputs.
- * inputFile: attribute should be set to an array containing the command line
- argument that should be used as the input file. Such array looks like
- this:
+ * inputFile: attribute should be set to an array containing the command
+ line argument that should be used as the input file. Such array looks
+ like this:
``["commandlinearg", "c", "default value" "The description"]``
- The second value of such arrray is the shorthand for the command line arg.
- The user will then be able to specify inputs to the test via:
+ The second value of such arrray is the shorthand for the command line
+ arg. The user will then be able to specify inputs to the test via:
``ooniprobe mytest.py --commandlinearg path/to/file.txt``
@@ -573,12 +597,14 @@ class NetTestCase(object):
* requiresRoot: set to True if the test must be run as root.
- * usageOptions: a subclass of twisted.python.usage.Options for processing of command line arguments
+ * usageOptions: a subclass of twisted.python.usage.Options for processing
+ of command line arguments
* localOptions: contains the parsed command line arguments.
Quirks:
- Every class that is prefixed with test *must* return a twisted.internet.defer.Deferred.
+ Every class that is prefixed with test *must* return a
+ twisted.internet.defer.Deferred.
"""
name = "This test is nameless"
author = "Jane Doe <foo at example.com>"
@@ -603,6 +629,7 @@ class NetTestCase(object):
requiresTor = False
localOptions = {}
+
def _setUp(self):
"""
This is the internal setup method to be overwritten by templates.
@@ -734,8 +761,8 @@ class NetTestCase(object):
for required_option in self.requiredOptions:
log.debug("Checking if %s is present" % required_option)
if required_option not in self.localOptions or \
- self.localOptions[required_option] == None:
- missing_options.append(required_option)
+ self.localOptions[required_option] is None:
+ missing_options.append(required_option)
if missing_options:
raise e.MissingRequiredOption(missing_options)
diff --git a/ooni/oonicli.py b/ooni/oonicli.py
index cc23ec5..f84ac2d 100644
--- a/ooni/oonicli.py
+++ b/ooni/oonicli.py
@@ -1,5 +1,3 @@
-#-*- coding: utf-8 -*-
-
import sys
import os
import yaml
@@ -17,6 +15,7 @@ from ooni.nettest import NetTestLoader
from ooni.utils import log, checkForRoot
+
class Options(usage.Options):
synopsis = """%s [options] [path to test].py
""" % (os.path.basename(sys.argv[0]),)
@@ -34,27 +33,27 @@ class Options(usage.Options):
["verbose", "v"]
]
- optParameters = [["reportfile", "o", None, "report file name"],
- ["testdeck", "i", None,
- "Specify as input a test deck: a yaml file containing the tests to run and their arguments"],
- ["collector", "c", None,
- "Address of the collector of test results. This option should not be used, but you should always use a bouncer."],
- ["bouncer", "b", 'httpo://nkvphnp3p6agi5qq.onion',
- "Address of the bouncer for test helpers. default: httpo://nkvphnp3p6agi5qq.onion"],
- ["logfile", "l", None, "log file name"],
- ["pcapfile", "O", None, "pcap file name"],
- ["configfile", "f", None,
- "Specify a path to the ooniprobe configuration file"],
- ["datadir", "d", None,
- "Specify a path to the ooniprobe data directory"],
- ["annotations", "a", None,
- "Annotate the report with a key:value[, key:value] format."]
- ]
+ optParameters = [
+ ["reportfile", "o", None, "report file name"],
+ ["testdeck", "i", None,
+ "Specify as input a test deck: a yaml file containing the tests to run and their arguments"],
+ ["collector", "c", None,
+ "Address of the collector of test results. This option should not be used, but you should always use a bouncer."],
+ ["bouncer", "b", 'httpo://nkvphnp3p6agi5qq.onion',
+ "Address of the bouncer for test helpers. default: httpo://nkvphnp3p6agi5qq.onion"],
+ ["logfile", "l", None, "log file name"],
+ ["pcapfile", "O", None, "pcap file name"],
+ ["configfile", "f", None,
+ "Specify a path to the ooniprobe configuration file"],
+ ["datadir", "d", None,
+ "Specify a path to the ooniprobe data directory"],
+ ["annotations", "a", None,
+ "Annotate the report with a key:value[, key:value] format."]]
compData = usage.Completions(
extraActions=[usage.CompleteFiles(
- "*.py", descr="file | module | package | TestCase | testMethod",
- repeat=True)],)
+ "*.py", descr="file | module | package | TestCase | testMethod",
+ repeat=True)],)
tracer = None
@@ -85,6 +84,7 @@ class Options(usage.Options):
except:
raise usage.UsageError("No test filename specified!")
+
def parseOptions():
print "WARNING: running ooniprobe involves some risk that varies greatly"
print " from country to country. You should be aware of this when"
@@ -94,12 +94,13 @@ def parseOptions():
cmd_line_options.getUsage()
try:
cmd_line_options.parseOptions()
- except usage.UsageError, ue:
+ except usage.UsageError as ue:
print cmd_line_options.getUsage()
- raise SystemExit, "%s: %s" % (sys.argv[0], ue)
+ raise SystemExit("%s: %s" % (sys.argv[0], ue))
return dict(cmd_line_options)
+
def runWithDirector(logging=True, start_tor=True):
"""
Instance the director, parse command line options and start an ooniprobe
@@ -122,9 +123,9 @@ def runWithDirector(logging=True, start_tor=True):
try:
checkForRoot()
except errors.InsufficientPrivileges:
- log.err("Insufficient Privileges to capture packets."
- " See ooniprobe.conf privacy.includepcap")
- sys.exit(2)
+ log.err("Insufficient Privileges to capture packets."
+ " See ooniprobe.conf privacy.includepcap")
+ sys.exit(2)
director = Director()
if global_options['list']:
@@ -157,10 +158,10 @@ def runWithDirector(logging=True, start_tor=True):
sys.exit(1)
global_options["annotations"] = annotations
- #XXX: This should mean no bouncer either!
+ # XXX: This should mean no bouncer either!
if global_options['no-collector']:
log.msg("Not reporting using a collector")
- collector = global_options['collector'] = None
+ global_options['collector'] = None
global_options['bouncer'] = None
deck = Deck()
@@ -178,16 +179,16 @@ def runWithDirector(logging=True, start_tor=True):
log.debug("No test deck detected")
test_file = nettest_to_path(global_options['test_file'], True)
net_test_loader = NetTestLoader(global_options['subargs'],
- test_file=test_file)
+ test_file=test_file)
deck.insert(net_test_loader)
- except errors.MissingRequiredOption, option_name:
+ except errors.MissingRequiredOption as option_name:
log.err('Missing required option: "%s"' % option_name)
print net_test_loader.usageOptions().getUsage()
sys.exit(2)
- except errors.NetTestNotFound, path:
+ except errors.NetTestNotFound as path:
log.err('Requested NetTest file not found (%s)' % path)
sys.exit(3)
- except usage.UsageError, e:
+ except usage.UsageError as e:
log.err(e)
print net_test_loader.usageOptions().getUsage()
sys.exit(4)
@@ -217,24 +218,29 @@ def runWithDirector(logging=True, start_tor=True):
log.err("Tor does not appear to be running")
log.err("Reporting with the collector %s is not possible" %
global_options['collector'])
- log.msg("Try with a different collector or disable collector reporting with -n")
+ log.msg(
+ "Try with a different collector or disable collector reporting with -n")
elif isinstance(failure.value, errors.InvalidOONIBCollectorAddress):
log.err("Invalid format for oonib collector address.")
- log.msg("Should be in the format http://<collector_address>:<port>")
+ log.msg(
+ "Should be in the format http://<collector_address>:<port>")
log.msg("for example: ooniprobe -c httpo://nkvphnp3p6agi5qq.onion")
elif isinstance(failure.value, errors.UnableToLoadDeckInput):
log.err("Unable to fetch the required inputs for the test deck.")
- log.msg("Please file a ticket on our issue tracker: https://github.com/thetorproject/ooni-probe/issues")
+ log.msg(
+ "Please file a ticket on our issue tracker: https://github.com/thetorproject/ooni-probe/issues")
elif isinstance(failure.value, errors.CouldNotFindTestHelper):
log.err("Unable to obtain the required test helpers.")
- log.msg("Try with a different bouncer or check that Tor is running properly.")
+ log.msg(
+ "Try with a different bouncer or check that Tor is running properly.")
elif isinstance(failure.value, errors.CouldNotFindTestCollector):
log.err("Could not find a valid collector.")
- log.msg("Try with a different bouncer, specify a collector with -c or disable reporting to a collector with -n.")
+ log.msg(
+ "Try with a different bouncer, specify a collector with -c or disable reporting to a collector with -n.")
elif isinstance(failure.value, errors.ProbeIPUnknown):
log.err("Failed to lookup probe IP address.")
@@ -267,7 +273,8 @@ def runWithDirector(logging=True, start_tor=True):
if not global_options['no-collector']:
if global_options['collector']:
collector = global_options['collector']
- elif 'collector' in config.reports and config.reports['collector']:
+ elif 'collector' in config.reports \
+ and config.reports['collector']:
collector = config.reports['collector']
elif net_test_loader.collector:
collector = net_test_loader.collector
diff --git a/ooni/reporter.py b/ooni/reporter.py
index 60df7eb..8341b1a 100644
--- a/ooni/reporter.py
+++ b/ooni/reporter.py
@@ -546,7 +546,8 @@ class Report(object):
def open_oonib_reporter(self):
def creation_failed(failure):
self.oonib_reporter = None
- return self.report_log.creation_failed(self.report_filename)
+ return self.report_log.creation_failed(self.report_filename,
+ self.collector_address)
def created(report_id):
return self.report_log.created(self.report_filename,
More information about the tor-commits
mailing list