commit 47b4587d22993343a26aef6eef3667fe959e005e Author: Arturo Filastò arturo@filasto.net Date: Wed Sep 14 17:03:21 2016 +0200
Address feedback from @bassosimone --- Vagrantfile | 5 ----- data/ooniprobe.conf.sample | 2 +- ooni/agent/scheduler.py | 24 +++++++++++++++++++----- ooni/deck/backend.py | 4 ++-- ooni/deck/deck.py | 2 +- ooni/director.py | 6 +++++- ooni/settings.py | 2 +- 7 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/Vagrantfile b/Vagrantfile index 8ff0bbd..71f7c7a 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -81,9 +81,4 @@ Vagrant.configure("2") do |config| end end
- config.vm.define "testing" do |testing| - testing.vm.network "forwarded_port", guest: 8842, host: 8142 - testing.vm.synced_folder ".", "/data/ooni-probe" - end - end diff --git a/data/ooniprobe.conf.sample b/data/ooniprobe.conf.sample index f4c0812..ee9a2c9 100644 --- a/data/ooniprobe.conf.sample +++ b/data/ooniprobe.conf.sample @@ -3,7 +3,7 @@ # Keep in mind that indentation matters.
basic: - # Where OONIProbe should be writing it's log file + # Where OONIProbe should be writing its log file logfile: /var/log/ooniprobe.log loglevel: WARNING privacy: diff --git a/ooni/agent/scheduler.py b/ooni/agent/scheduler.py index 2797fba..c86e3b6 100644 --- a/ooni/agent/scheduler.py +++ b/ooni/agent/scheduler.py @@ -24,9 +24,12 @@ from ooni.measurements import list_measurements class FileSystemlockAndMutex(object): """ This is a lock that is both a mutex lock and also on filesystem. - When you acquire it, it will first block on the mutex lock and then - once that is released it will attempt to acquire the lock on the - filesystem. + When you acquire it, it will first acquire the mutex lock and then + acquire the filesystem lock. The release order is inverted. + + This is to avoid concurrent usage within the same process. When using it + concurrently the mutex lock will block before the filesystem lock is + acquired.
It's a way to support concurrent usage of the DeferredFilesystemLock without races. @@ -291,7 +294,7 @@ class SendHeartBeat(ScheduledTask): # XXX implement this pass
-# Order mattters +# Order matters SYSTEM_TASKS = [ UpdateInputsAndResources ] @@ -303,7 +306,10 @@ def run_system_tasks(no_input_store=False):
if no_input_store: log.debug("Not updating the inputs") - task_classes.remove(UpdateInputsAndResources) + try: + task_classes.remove(UpdateInputsAndResources) + except ValueError: + pass
for task_class in task_classes: task = task_class() @@ -337,7 +343,15 @@ class SchedulerService(service.MultiService): self._scheduled_tasks.remove(task)
def refresh_deck_list(self): + """ + This checks if there are some decks that have been enabled and + should be scheduled as periodic tasks to run on the next scheduler + cycle and if some have been disabled and should not be run.
+ It does so by listing the enabled decks and checking if the enabled + ones are already scheduled or if some of the scheduled ones are not + amongst the enabled decks. + """ to_enable = [] for deck_id, deck in deck_store.list_enabled(): if deck.schedule is None: diff --git a/ooni/deck/backend.py b/ooni/deck/backend.py index b2df9bc..a39492c 100644 --- a/ooni/deck/backend.py +++ b/ooni/deck/backend.py @@ -16,6 +16,8 @@ def sort_addresses_by_priority(priority_address, 'address': priority_address, 'type': backend_type } + # We prefer an onion collector to an https collector to a cloudfront + # collector to a plaintext collector address_priority = ['onion', 'https', 'cloudfront', 'http'] address_priority.remove(preferred_backend) address_priority.insert(0, preferred_backend) @@ -73,8 +75,6 @@ def get_reachable_test_helper(test_helper_name, test_helper_address, @defer.inlineCallbacks def get_reachable_collector(collector_address, collector_alternate, preferred_backend): - # We prefer onion collector to https collector to cloudfront - # collectors to plaintext collectors for collector_settings in sort_addresses_by_priority( collector_address, collector_alternate, diff --git a/ooni/deck/deck.py b/ooni/deck/deck.py index 3fc10ab..59946ab 100644 --- a/ooni/deck/deck.py +++ b/ooni/deck/deck.py @@ -25,7 +25,7 @@ def resolve_file_path(v, prepath=None): if v.startswith("$"): # This raises InputNotFound and we let it carry onto the caller return input_store.get(v[1:])["filepath"] - elif prepath is not None and (not os.path.isabs(v)): + if prepath is not None and (not os.path.isabs(v)): return FilePath(prepath).preauthChild(v).path return v
diff --git a/ooni/director.py b/ooni/director.py index 1807e65..1e55bb6 100644 --- a/ooni/director.py +++ b/ooni/director.py @@ -112,7 +112,11 @@ class Director(object):
def notify(self, event): for handler in self._subscribers: - handler(event) + try: + handler(event) + except Exception as exc: + log.err("Failed to run handler") + log.exception(exc)
def _reset_director_state(self): self._director_state = 'not-running' diff --git a/ooni/settings.py b/ooni/settings.py index 14f135f..b4bb594 100644 --- a/ooni/settings.py +++ b/ooni/settings.py @@ -18,7 +18,7 @@ CONFIG_FILE_TEMPLATE = """\ # Keep in mind that indentation matters.
basic: - # Where OONIProbe should be writing it's log file + # Where OONIProbe should be writing its log file # logfile: {logfile} # loglevel: WARNING # The maximum amount of data to store on disk. Once the quota is reached,