Skip to content

Commit

Permalink
fixed the possible bug in not stripping enough whitespace in the jobs…
Browse files Browse the repository at this point in the history
… option
  • Loading branch information
peterbe committed Jul 13, 2012
1 parent 81002db commit 773e016
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
21 changes: 17 additions & 4 deletions socorro/cron/crontabber.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ def load(self, file_path):
super(JSONAndPostgresJobDatabase, self).load(file_path)

def _load_from_postgres(self, file_path):
database_class = self.config.database.database_class(self.config.database)
database_class = self.config.database.database_class(
self.config.database
)
with database_class() as connection:
cursor = connection.cursor()
cursor.execute('SELECT state FROM crontabber_state')
Expand Down Expand Up @@ -479,6 +481,15 @@ def check_time(value):
raise TimeDefinitionError("Invalid definition of time %r" % value)


def line_splitter(text):
return [x.strip() for x in text.splitlines()
if x.strip()]


def pipe_splitter(text):
return text.split('|', 1)[0]


class CronTabber(App):

app_name = 'crontabber'
Expand Down Expand Up @@ -506,8 +517,8 @@ class CronTabber(App):
from_string_converter=
classes_in_namespaces_converter_with_compression(
reference_namespace=required_config.crontabber,
list_splitter_fn=lambda x: x.splitlines(),
class_extractor=lambda x: x.split('|', 1)[0],
list_splitter_fn=line_splitter,
class_extractor=pipe_splitter,
extra_extractor=get_extra_as_options
)
)
Expand Down Expand Up @@ -575,7 +586,9 @@ def main(self):
@property
def database(self):
if not getattr(self, '_database', None):
self._database = self.config.crontabber.json_database_class(self.config)
self._database = self.config.crontabber.json_database_class(
self.config
)
self._database.load(self.config.crontabber.database_file)
return self._database

Expand Down
25 changes: 20 additions & 5 deletions socorro/unittest/cron/test_crontabber.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,16 @@
import re
import sys
import datetime
import shutil
import os
import json
import unittest
import tempfile
from cStringIO import StringIO
import mock
import psycopg2
from psycopg2.extensions import TRANSACTION_STATUS_IDLE
from nose.plugins.attrib import attr
from socorro.cron import crontabber
from socorro.lib.datetimeutil import utc_now
from configman import ConfigurationManager, Namespace
from configman import Namespace
from .base import DSN, TestCaseBase


Expand Down Expand Up @@ -615,7 +612,8 @@ def test_own_required_config_job_overriding_config(self):
config_manager, json_file = self._setup_config_manager(
'socorro.unittest.cron.test_crontabber.OwnRequiredConfigSampleJob|1d',
extra_value_source={
'crontabber.class-OwnRequiredConfigSampleJob.bugsy_url': 'bugs.peterbe.com'
'crontabber.class-OwnRequiredConfigSampleJob.bugsy_url':
'bugs.peterbe.com'
}
)

Expand Down Expand Up @@ -787,6 +785,23 @@ def fmt(d):
self.assertTrue([x for x in infos
if formatted in x])

def test_run_with_excess_whitespace(self):
# this test asserts a found bug where excess newlines
# caused configuration exceptions
config_manager, json_file = self._setup_config_manager(
'\n \n'
' socorro.unittest.cron.test_crontabber.BasicJob|7d\n\t \n'
)

with config_manager.context() as config:
tab = crontabber.CronTabber(config)
tab.run_all()

structure = json.load(open(json_file))
information = structure['basic-job']
self.assertTrue(information['last_success'])
self.assertTrue(not information['last_error'])


#==============================================================================
@attr(integration='postgres') # for nosetests
Expand Down

0 comments on commit 773e016

Please sign in to comment.