From 18520efbfd2b8d641ed2285ac53ff4162a095380 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Tue, 1 Oct 2013 14:44:27 -0700 Subject: [PATCH 1/2] Fixes for SqlAlchemy/python nonsense * Created strToBool to convert config string to bool ** Created it twice because circular dependency. * Moved Counter from metrics because SqlAlchemy wasn't creating the table. * dropped util/utils because it was confusing (thus introducing the circle) * fixed unit tests to handle lack of whitelist user table ** Fixed settings elements to check correct source. * swore. A lot. --- campaign/__init__.py | 14 +++-- campaign/decorators.py | 16 +++-- campaign/logger.py | 4 +- campaign/storage/metrics.py | 116 ----------------------------------- campaign/storage/sql.py | 111 ++++++++++++++++++++++++++++++++- campaign/templates/main.mako | 2 +- campaign/tests/test_views.py | 18 +++--- campaign/util.py | 15 ----- campaign/utils.py | 17 +++++ campaign/views.py | 31 ++++++---- setup.py | 2 +- 11 files changed, 178 insertions(+), 168 deletions(-) delete mode 100644 campaign/storage/metrics.py diff --git a/campaign/__init__.py b/campaign/__init__.py index 5a046c7..fc715da 100644 --- a/campaign/__init__.py +++ b/campaign/__init__.py @@ -8,11 +8,16 @@ from mozsvc.config import load_into_settings from mozsvc.middlewares import _resolve_name from campaign.logger import Logging, LOG -from campaign.storage.metrics import Counter logger = None counter = None +# TO prevent circular references, duplicate this func here. +def strToBool(s="False"): + if type(s) == bool: + return s + return s.lower() in ["true", "1", "yes", "t"] + def get_group(group_name, dictionary): if group_name is None: @@ -66,7 +71,6 @@ def main(global_config, **settings): config.add_static_view(name='static', path='campaign:static') config.scan("campaign.views") logger = Logging(config, global_config['__file__']) - counter = Counter(config=config, logger=logger) config.registry['storage'] = _resolve_name( settings.get('db.backend', 'campaign.storage.sql.Storage'))(config=config, @@ -75,11 +79,9 @@ def main(global_config, **settings): 'auth', settings['config'].get_map('auth')) config.registry['logger'] = logger - config.registry['counter'] = counter - if settings.get('dbg.self_diag', False): + config.registry['counter'] = config.registry['storage'].counter + if strToBool(settings.get('dbg.self_diag', False)): self_diag(config) config.registry['logger'].log('Starting up', fields='', severity=LOG.INFORMATIONAL) return config.make_wsgi_app() - - diff --git a/campaign/decorators.py b/campaign/decorators.py index 53e3124..6c76020 100644 --- a/campaign/decorators.py +++ b/campaign/decorators.py @@ -1,6 +1,7 @@ import json import pyramid.httpexceptions as http import random +import utils from campaign import logger, LOG from campaign.auth.default import DefaultAuth from dateutil import parser @@ -62,14 +63,18 @@ def authorized(self, email, request): if not result: return False storage = request.registry.get('storage') - return storage.is_user(email) + if settings.get("db.checkAccount", True): + return storage.is_user(email) + else: + return True except TypeError: pass except Exception: - if settings.get('dbg.traceback', False): + if utils.strToBool(settings.get('dbg.traceback', False)): import traceback traceback.print_exc() - if settings.get('dbg.break_unknown_exception', False): + if utils.strToBool(settings.get('dbg.break_unknown_exception', + False)): import pdb pdb.set_trace() pass @@ -107,10 +112,11 @@ def login(self, request, skipAuth=False): raise e except Exception, e: settings = request.registry.settings - if settings.get('dbg.traceback', False): + if utils.strToBool(settings.get('dbg.traceback', False)): import traceback traceback.print_exc() - if settings.get('dbg.break_unknown_exception', False): + if utils.strToBool(settings.get('dbg.break_unknown_exception', + False)): import pdb pdb.set_trace() logger.log(type='error', severity=LOG.ERROR, msg=str(e)) diff --git a/campaign/logger.py b/campaign/logger.py index d11275b..43b5bd4 100644 --- a/campaign/logger.py +++ b/campaign/logger.py @@ -1,5 +1,6 @@ import logging import json +import campaign.utils as utils from __builtin__ import type as btype @@ -28,6 +29,7 @@ class LoggingException(Exception): class Logging(object): + metlog2log = [logging.CRITICAL, logging.CRITICAL, logging.CRITICAL, logging.ERROR, logging.WARNING, logging.INFO, logging.INFO, logging.DEBUG] @@ -42,7 +44,7 @@ def __init__(self, config, settings_file): self.loggername = settings.get('logging.name', 'campaign-manager') self.logger = logging.getLogger(self.loggername) self.logger.level = 1 - if HEKA and settings.get('logging.use_heka', True): + if HEKA and utils.strToBool(settings.get('logging.use_heka', "true")): self.heka = client_from_stream_config( open(settings_file, 'r'), 'heka') diff --git a/campaign/storage/metrics.py b/campaign/storage/metrics.py deleted file mode 100644 index 4bdb990..0000000 --- a/campaign/storage/metrics.py +++ /dev/null @@ -1,116 +0,0 @@ -from campaign.logger import LOG -from campaign.storage import StorageBase -from datetime import datetime -from sqlalchemy import (Column, Integer, String, - MetaData, text) -from sqlalchemy.ext.declarative import declarative_base -import json -import re -import time - - -Base = declarative_base() - - -class Scrapes(Base): - __tablename__ = 'scrapes' - - id = Column('id', String(25), unique=True, primary_key=True) - served = Column('served', Integer, server_default=text('0')) - clicks = Column('clicks', Integer, server_default=text('0')) - last = Column('last', Integer, index=True, server_default=text('0')) - - -class CounterException(Exception): - pass - - -class Counter(StorageBase): - __database__ = 'campaign' - __tablename__ = 'scrapes' - - def __init__(self, config, logger, **kw): - try: - super(Counter, self).__init__(config, **kw) - self.logger = logger - self.metadata = MetaData() - self._connect() - #TODO: add the most common index. - except Exception, e: - logger.log(msg='Could not initialize Storage "%s"' % str(e), - type='error', severity=LOG.CRITICAL) - raise e - - def bulk_increment(self, conn, id, action, time=time.time()): - action = re.sub(r'[^0-9A-Za-z]', '', action) - try: - if (self.settings.get("db.type") == "sqlite"): - conn.execute(text("insert or ignore into " + - self.__tablename__ + - " (id)" + - " values (:id ); "), - {"id": id}) - else: - dml = text("insert into " + self.__tablename__ - + " (id, %s) values (:id, 1) " % action - + " on duplicate key update %s=%s+1, last=:last;" - % (action, action)) - conn.execute(dml, {"id": id, "last": time}) - except Exception, e: - self.logger.log(msg="Could not increment id: %s" % str(e), - type="error", severity=LOG.ERROR) - - def increment(self, id, action, time): - with self.engine.begin() as conn: - self.bulk_increment(conn, id, action, time) - - def fetched(self, data, time=time.time()): - with self.engine.begin() as conn: - for item in data: - self.bulk_increment(conn, item.get('token'), 'served', time) - - def redir(self, data, time=time.time()): - self.increment(data.get('id'), 'clicks', time) - - commands = {'redirect': redir, - 'fetched': fetched} - - def log(self, line): - for command in self.commands.keys(): - if command + ' :' in line: - dt = datetime.strptime(line.split(',')[0], - '%Y-%m-%d %H:%M:%S') - timestamp = int(time.mktime(dt.timetuple())) - try: - data = json.loads(line.split(command + ' :')[1]) - while (isinstance(data, basestring)): - data = json.loads(data) - self.commands.get(command)(self, - data, - timestamp) - except Exception, e: - self.logger.log(msg="Could not log %s" % str(e), - type="error", severity=LOG.ERROR) - raise e - - def report(self, id): - with self.engine.begin() as conn: - resp = conn.execute(text(("select * from %s " % - self.__tablename__) + - "where id = :id"), {'id': id}) - if resp.rowcount > 0: - result = resp.fetchone() - return dict(zip(resp.keys(), result)) - else: - return {} - - def parse(self, logfile): - try: - file = open(logfile, 'r') - for line in file: - self.log(line) - except Exception, e: - self.logger.log(msg="Could not parse %s" % str(e), - type="error", severity=LOG.ERROR) - pass - diff --git a/campaign/storage/sql.py b/campaign/storage/sql.py index 9660cc3..36e9d1b 100644 --- a/campaign/storage/sql.py +++ b/campaign/storage/sql.py @@ -1,11 +1,13 @@ import json import time +import datetime import uuid +import re from . import StorageBase, StorageException, Base -from .metrics import Counter from campaign.logger import LOG from campaign.views import api_version -from sqlalchemy import (Column, Integer, String, Text, text) +from sqlalchemy import (Column, Integer, String, Text, + text) class Users(Base): @@ -43,6 +45,108 @@ class Campaign(Base): status = Column('status', Integer) +class Scrapes(Base): + __tablename__ = 'scrapes' + + id = Column('id', String(25), unique=True, primary_key=True) + served = Column('served', Integer, server_default=text('0')) + clicks = Column('clicks', Integer, server_default=text('0')) + last = Column('last', Integer, index=True, server_default=text('0')) + + +class CounterException(Exception): + pass + + +class Counter(StorageBase): + __database__ = 'campaign' + __tablename__ = 'scrapes' + + def __init__(self, config, logger, **kw): + try: + super(Counter, self).__init__(config, **kw) + self.logger = logger + self._connect() + #TODO: add the most common index. + except Exception, e: + logger.log(msg='Could not initialize Storage "%s"' % str(e), + type='error', severity=LOG.CRITICAL) + raise e + + def bulk_increment(self, conn, id, action, time=time.time()): + action = re.sub(r'[^0-9A-Za-z]', '', action) + try: + if (self.settings.get("db.type") == "sqlite"): + conn.execute(text("insert or ignore into " + + self.__tablename__ + + " (id)" + + " values (:id ); "), + {"id": id}) + else: + dml = text("insert into " + self.__tablename__ + + " (id, %s) values (:id, 1) " % action + + " on duplicate key update %s=%s+1, last=:last;" + % (action, action)) + conn.execute(dml, {"id": id, "last": time}) + except Exception, e: + self.logger.log(msg="Could not increment id: %s" % str(e), + type="error", severity=LOG.ERROR) + + def increment(self, id, action, time): + with self.engine.begin() as conn: + self.bulk_increment(conn, id, action, time) + + def fetched(self, data, time=time.time()): + with self.engine.begin() as conn: + for item in data: + self.bulk_increment(conn, item.get('token'), 'served', time) + + def redir(self, data, time=time.time()): + self.increment(data.get('id'), 'clicks', time) + + commands = {'redirect': redir, + 'fetched': fetched} + + def log(self, line): + for command in self.commands.keys(): + if command + ' :' in line: + dt = datetime.strptime(line.split(',')[0], + '%Y-%m-%d %H:%M:%S') + timestamp = int(time.mktime(dt.timetuple())) + try: + data = json.loads(line.split(command + ' :')[1]) + while (isinstance(data, basestring)): + data = json.loads(data) + self.commands.get(command)(self, + data, + timestamp) + except Exception, e: + self.logger.log(msg="Could not log %s" % str(e), + type="error", severity=LOG.ERROR) + raise e + + def report(self, id): + with self.engine.begin() as conn: + resp = conn.execute(text(("select * from %s " % + self.__tablename__) + + "where id = :id"), {'id': id}) + if resp.rowcount > 0: + result = resp.fetchone() + return dict(zip(resp.keys(), result)) + else: + return {} + + def parse(self, logfile): + try: + file = open(logfile, 'r') + for line in file: + self.log(line) + except Exception, e: + self.logger.log(msg="Could not parse %s" % str(e), + type="error", severity=LOG.ERROR) + pass + + class Storage(StorageBase): __database__ = 'campaign' __tablename__ = 'campaigns' @@ -51,11 +155,12 @@ def __init__(self, config, logger, **kw): try: super(Storage, self).__init__(config, **kw) self.logger = logger - self._connect() # Store off a link to the main table. self.campaigns = Base.metadata.tables.get(Campaign.__tablename__) self.users = Base.metadata.tables.get(Users.__tablename__) + self.scrapes = Base.metadata.tables.get(Scrapes.__tablename__) self.counter = Counter(config, logger, **kw) + self._connect() #TODO: add the most common index. except Exception, e: logger.log(msg='Could not initialize Storage "%s"' % str(e), diff --git a/campaign/templates/main.mako b/campaign/templates/main.mako index 4eb980e..19320a7 100644 --- a/campaign/templates/main.mako +++ b/campaign/templates/main.mako @@ -1,6 +1,6 @@ <% - from campaign.util import strToUTC + from campaign.utils import strToUTC from time import (time, strftime, gmtime) import json diff --git a/campaign/tests/test_views.py b/campaign/tests/test_views.py index e858f91..fc72072 100644 --- a/campaign/tests/test_views.py +++ b/campaign/tests/test_views.py @@ -1,6 +1,5 @@ from campaign import views from campaign.storage.sql import Storage -from campaign.storage.metrics import Counter from campaign.tests import TConfig from campaign.logger import Logging from nose.tools import eq_ @@ -58,6 +57,11 @@ class ViewTest(unittest2.TestCase): 'body': "\u0192\u00a9\u02d9\u02da\u02da\u00ac\u2206\u02d9\u02da\u02d9\u00a9\u2206\u00a9\u0192\u00df \u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376\u0376"} ] + settings = TConfig({'db.type': 'sqlite', + 'db.db': '/tmp/test.db', + 'logging.use_heka': False, + 'db.checkAccount': False}) + def req(self, matchdict={}, user_id=None, headers=None, **kw): class Reg(dict): @@ -70,25 +74,23 @@ def __init__(self, settings=None, **kw): self.settings = settings request = Request(headers=headers, **kw) - request.GET = kw.get('params',{}) + request.GET = kw.get('params', {}) request.registry = Reg(settings=self.config.get_settings()) request.registry['storage'] = self.storage request.registry['logger'] = self.logger request.registry['counter'] = self.counter request.registry['auth'] = mock.Mock() request.registry['auth'].get_user_id.return_value = user_id + request.registry.settings.update(self.settings.settings) if matchdict: request.matchdict.update(matchdict) return request def setUp(self): self.config = testing.setUp() - tsettings = TConfig({'db.type': 'sqlite', - 'db.db': '/tmp/test.db', - 'logging.use_heka': False}) - self.logger = Logging(tsettings, None) - self.storage = Storage(config=tsettings, logger=self.logger) - self.counter = Counter(config=tsettings, logger=self.logger) + self.logger = Logging(self.settings, None) + self.storage = Storage(config=self.settings, logger=self.logger) + self.counter = self.storage.counter records = [] for diff in self.diffs: record = self.base_record.copy() diff --git a/campaign/util.py b/campaign/util.py index da19d3b..139597f 100644 --- a/campaign/util.py +++ b/campaign/util.py @@ -1,17 +1,2 @@ -from email import utils as eut -from . import logger, LOG -import calendar -def strToUTC(datestr): - secs = 0 - try: - timet = eut.parsedate_tz(datestr) - secs = int(calendar.timegm(timet[:8])) + timet[9] - except Exception, e: - if logger: - logger.log(type='campaign', - severity=LOG.ERROR, - msg=repr(e) + str(datestr), - fields={}) - return secs diff --git a/campaign/utils.py b/campaign/utils.py index f1a7284..67f803b 100644 --- a/campaign/utils.py +++ b/campaign/utils.py @@ -1,11 +1,22 @@ # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +#from campaign import logger, LOG +from email import utils as eut +import calendar import string ALPHABET = (string.digits + string.letters) +def strToUTC(datestr): + # Logging and trapping this error causes circular dependencies + secs = 0 + timet = eut.parsedate_tz(datestr) + secs = int(calendar.timegm(timet[:8])) + timet[9] + return secs + + def as_id(num): if num == 0: return ALPHABET[0] @@ -29,3 +40,9 @@ def from_id(id): num += ALPHABET.index(char) * (base ** power) idx += 1 return num + + +def strToBool(s="False"): + if type(s) == bool: + return s + return s.lower() in ["true", "1", "yes", "t"] diff --git a/campaign/views.py b/campaign/views.py index 48538b6..dff6d75 100644 --- a/campaign/views.py +++ b/campaign/views.py @@ -4,16 +4,16 @@ """ Cornice services. """ from campaign.logger import LOG -from campaign.util import strToUTC from decorators import checkService, authorizedOnly from mako.template import Template from mozsvc.metrics import Service from webob import Response +import campaign.utils as utils import json import os import pyramid.httpexceptions as http import time -import pprint +import logger api_version = 1 @@ -72,7 +72,13 @@ def get_last_accessed(request): try: if 'If-Modified-Since' in request.headers: last_accessed_str = request.headers.get('If-Modified-Since') - last_accessed = strToUTC(last_accessed_str) + try: + last_accessed = utils.strToUTC(last_accessed_str) + except Exception, e: + request.registry['logger'].log(type='error', + severity=LOG.ERROR, + msg='Exception: %e' % str(e)) + last_accessed = 0 # pop off tz adjustment (in seconds) if request.registry['logger']: ims_str = time.strftime('%a, %d %b %Y %H:%M:%S GMT', @@ -85,10 +91,10 @@ def get_last_accessed(request): fields={}) except Exception, e: settings = request.registry.settings - if settings.get('dbg.traceback', False): + if utils.strToBool(settings.get('dbg.traceback', False)): import traceback traceback.print_exc() - if settings.get('dbg.break_unknown_exception', False): + if utils.strToBool(settings.get('dbg.break_unknown_exception', False)): import pdb pdb.set_trace() request.registry['logger'].log(type='error', @@ -200,7 +206,8 @@ def get_all_announcements(request): @author.get() @author2.get() def admin_page(request, error=None): - if request.registry.settings.get('auth.block_authoring', False): + if utils.strToBool(request.registry.settings.get('auth.block_authoring', + False)): raise http.HTTPNotFound() auth = authorizedOnly(None) if not auth.login(request): @@ -230,7 +237,8 @@ def admin_page(request, error=None): def manage_announce(request): args = request.params.copy() args.update(request.matchdict) - if request.registry.settings.get('auth.block_authoring', False): + if utils.strToBool(request.registry.settings.get('auth.block_authoring', + False)): raise http.HTTPNotFound() # Clean up the login info try: @@ -256,10 +264,10 @@ def manage_announce(request): args['author'] = session.get('uid') storage.put_announce(args) except Exception, e: - if settings.get('dbg.traceback', False): + if utils.strToBool(settings.get('dbg.traceback', False)): import traceback traceback.print_exc() - if settings.get('dbg.break_unknown_exception', False): + if utils.strToBool(settings.get('dbg.break_unknown_exception', False)): import pdb pdb.set_trace() # display error page. @@ -325,10 +333,10 @@ def login_page(request, error=None): return response except Exception, e: settings = request.registry.settings - if settings.get('dbg.traceback', False): + if utils.strToBool(settings.get('dbg.traceback', False)): import traceback traceback.print_exc() - if settings.get('dbg.break_unknown_exception', False): + if utils.strToBool(settings.get('dbg.break_unknown_exception', False)): import pdb pdb.set_trace() request.registry['logger'].log(str(e), type='error', @@ -376,7 +384,6 @@ def admin_get(request, error=None): content_type=content_type) return response except Exception, e: - import pdb; pdb.set_trace() print e diff --git a/setup.py b/setup.py index fee704e..778d70e 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ setup(name='campaign', - version=1.000, + version=1.001, description='campaign', long_description=README, classifiers=[ From 874a5c2c0d43fe719dd856e44d85fe93f0d80a52 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Wed, 2 Oct 2013 14:58:40 -0700 Subject: [PATCH 2/2] fix to patch because sqlalchemy is awesome. --- sql/20130927_sql_patch.sql | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sql/20130927_sql_patch.sql b/sql/20130927_sql_patch.sql index 04d1481..0025260 100644 --- a/sql/20130927_sql_patch.sql +++ b/sql/20130927_sql_patch.sql @@ -14,7 +14,14 @@ CREATE TABLE if not exists `users` ( LOCK TABLES `users` WRITE; /*!40000 ALTER TABLE `users` DISABLE KEYS */; -INSERT INTO `users` VALUES ('0','jconlin@mozilla.com',NULL,1380154102,0),('1085eb386c2940a9a59c8f93e7655a41','bwong@mozilla.com','jconlin@mozilla.com',1380238317,0),('91becec273e14ff3ae4228fc3d0a6509','jmontano@mozilla.com','jconlin@mozilla.com',1380238334,0),('a91f69ef18b04d018470d8ca28d0997e','elancaster@mozilla.com','jconlin@mozilla.com',1380238948,0) On Duplicate Key Update `date` = unix_timestamp(); +INSERT INTO `users` (`id`, `email`, `sponsor`, `date`, `level`) VALUES +('0','jconlin@mozilla.com',NULL,1380154102,0),('1085eb386c2940a9a59c8f93e7655a41','bwong@mozilla.com','jconlin@mozilla.com',1380238317,0), +('91becec273e14ff3ae4228fc3d0a6509','jmontano@mozilla.com','jconlin@mozilla.com',1380238334,0), +('a91f69ef18b04d018470d8ca28d0997e','elancaster@mozilla.com','jconlin@mozilla.com',1380238948,0), +('3c1340f20e20489ab6f4399d958eb483','edwong@mozilla.com','jconlin@mozilla.com', 1380238317, 0), +('95a8d2d4da1445ba959e3dedb1fe712b','oremj@mozilla.com','jconlin@mozilla.com', 1380238317, 0) +On Duplicate Key Update `date` = unix_timestamp(); + /*!40000 ALTER TABLE `users` ENABLE KEYS */; UNLOCK TABLES;