Milimetric has uploaded a new change for review. https://gerrit.wikimedia.org/r/102462
Change subject: making sure all session.close calls happen ...................................................................... making sure all session.close calls happen Change-Id: Ib506c1a6cca5b13aaa957de98bfa837860cd5e4a --- M wikimetrics/config/web_config.yaml M wikimetrics/controllers/authentication.py M wikimetrics/controllers/demo.py M wikimetrics/controllers/reports.py M wikimetrics/database.py M wikimetrics/models/cohort.py M wikimetrics/models/report_nodes/report.py M wikimetrics/models/report_nodes/run_report.py 8 files changed, 95 insertions(+), 72 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/analytics/wikimetrics refs/changes/62/102462/1 diff --git a/wikimetrics/config/web_config.yaml b/wikimetrics/config/web_config.yaml index 19a270e..207593e 100644 --- a/wikimetrics/config/web_config.yaml +++ b/wikimetrics/config/web_config.yaml @@ -23,6 +23,7 @@ META_MW_TOKEN_URI : 'w/index.php?title=Special:OAuth/token' META_MW_AUTH_URI : 'wiki/Special:OAuth/authorize' META_MW_USERINFO_URI : 'w/api.php?action=query&meta=userinfo&format=json' +#META_MW_IDENTIFY_URI : 'w/index.php?title=Special:OAuth/identify&format=json' META_MW_REDIRECT_URI : 'http://localhost:5000/auth/meta_mw' META_MW_CONSUMER_KEY : 'bad4e459823278bdffb5ecf0a206112d' META_MW_CLIENT_SECRET : 'e312699be56f1d157657727a87ce3776e172501a' diff --git a/wikimetrics/controllers/authentication.py b/wikimetrics/controllers/authentication.py index d45f0c0..3e8630e 100644 --- a/wikimetrics/controllers/authentication.py +++ b/wikimetrics/controllers/authentication.py @@ -55,8 +55,10 @@ Callback required by Flask-Login. Gets the User object from the database. """ db_session = db.get_session() - user = User.get(db_session, user_id) - db_session.close() + try: + user = User.get(db_session, user_id) + finally: + db_session.close() return user @@ -79,9 +81,11 @@ """ session['access_token'] = None db_session = db.get_session() - if type(current_user) is User: - current_user.logout(db_session) - db_session.close() + try: + if type(current_user) is User: + current_user.logout(db_session) + finally: + db_session.close() logout_user() return redirect(url_for('home_index')) @@ -224,8 +228,8 @@ db_session.close() return 'Multiple users found with your id!!! Contact Administrator' - user.login(db_session) try: + user.login(db_session) if login_user(user): user.detach_from(db_session) redirect_to = session.get('next') or url_for('home_index') @@ -250,8 +254,10 @@ def login_for_testing_only(): if app.config['DEBUG']: db_session = db.get_session() - user = db_session.query(User).filter_by(email='[email protected]').one() - user.login(db_session) - login_user(user) - db_session.close() + try: + user = db_session.query(User).filter_by(email='[email protected]').one() + user.login(db_session) + login_user(user) + finally: + db_session.close() return '' diff --git a/wikimetrics/controllers/demo.py b/wikimetrics/controllers/demo.py index 7a248cf..d00b5a1 100644 --- a/wikimetrics/controllers/demo.py +++ b/wikimetrics/controllers/demo.py @@ -57,14 +57,15 @@ @app.route('/demo/metric/random/<int:cohort_id>') def run_task_in_celery(cohort_id): db_session = db.get_session() - user_ids = db_session.query(WikiUser.mediawiki_userid)\ - .join(CohortWikiUser)\ - .filter(CohortWikiUser.cohort_id == cohort_id)\ - .all() - if len(user_ids) == 0: - user_ids = db_session.query(WikiUser.mediawiki_userid).all() - - db_session.close() + try: + user_ids = db_session.query(WikiUser.mediawiki_userid)\ + .join(CohortWikiUser)\ + .filter(CohortWikiUser.cohort_id == cohort_id)\ + .all() + if len(user_ids) == 0: + user_ids = db_session.query(WikiUser.mediawiki_userid).all() + finally: + db_session.close() report = MetricReport(RandomMetric(), user_ids, 'enwiki') #from nose.tools import set_trace; set_trace() @@ -138,8 +139,6 @@ ) # TODO: these users don't actually exist in the mediawiki databases, add them - #db_enwiki_session = db.get_mw_session('enwiki') - #db_dewiki_session = db.get_mw_session('dewiki') wu1 = WikiUser(mediawiki_username='Dan', mediawiki_userid=1, project='enwiki') wu2 = WikiUser(mediawiki_username='Evan', mediawiki_userid=2, project='enwiki') wu3 = WikiUser(mediawiki_username='Andrew', mediawiki_userid=3, project='enwiki') @@ -281,21 +280,23 @@ @app.route('/demo/create/fake-<string:project>-users/<int:n>') def add_fake_enwiki_users(project, n): session = db.get_mw_session(project) - max_id = session.query(func.max(MediawikiUser.user_id)).one()[0] or 0 - start = max_id + 1 - session.bind.engine.execute( - MediawikiUser.__table__.insert(), - [ - { - 'user_name' : 'user-{0}'.format(r), - 'user_id' : r, - 'user_registration' : '20130101000000' - } - for r in range(start, start + n) - ] - ) - session.commit() - session.close() + try: + max_id = session.query(func.max(MediawikiUser.user_id)).one()[0] or 0 + start = max_id + 1 + session.bind.engine.execute( + MediawikiUser.__table__.insert(), + [ + { + 'user_name' : 'user-{0}'.format(r), + 'user_id' : r, + 'user_registration' : '20130101000000' + } + for r in range(start, start + n) + ] + ) + session.commit() + finally: + session.close() return '{0} user records created in {1}'.format(n, project) diff --git a/wikimetrics/controllers/reports.py b/wikimetrics/controllers/reports.py index eb1751d..4f47c17 100644 --- a/wikimetrics/controllers/reports.py +++ b/wikimetrics/controllers/reports.py @@ -42,19 +42,21 @@ @app.route('/reports/list/') def reports_list(): db_session = db.get_session() - reports = db_session.query(PersistentReport)\ - .filter(PersistentReport.user_id == current_user.id)\ - .filter(PersistentReport.created > thirty_days_ago())\ - .filter(PersistentReport.show_in_ui)\ - .all() - # TODO: update status for all reports at all times (not just show_in_ui ones) - # update status for each report - for report in reports: - report.update_status() - - # TODO fix json_response to deal with PersistentReport objects - reports_json = json_response(reports=[report._asdict() for report in reports]) - db_session.close() + try: + reports = db_session.query(PersistentReport)\ + .filter(PersistentReport.user_id == current_user.id)\ + .filter(PersistentReport.created > thirty_days_ago())\ + .filter(PersistentReport.show_in_ui)\ + .all() + # TODO: update status for all reports at all times (not just show_in_ui ones) + # update status for each report + for report in reports: + report.update_status() + + # TODO fix json_response to deal with PersistentReport objects + reports_json = json_response(reports=[report._asdict() for report in reports]) + finally: + db_session.close() return reports_json @@ -74,12 +76,14 @@ try: db_session = db.get_session() - pj = db_session.query(PersistentReport)\ - .filter(PersistentReport.result_key == result_key)\ - .one() - - celery_task = Report.task.AsyncResult(pj.queue_result_key) - db_session.close() + try: + pj = db_session.query(PersistentReport)\ + .filter(PersistentReport.result_key == result_key)\ + .one() + + celery_task = Report.task.AsyncResult(pj.queue_result_key) + finally: + db_session.close() return (celery_task, pj) except NoResultFound: return (None, None) diff --git a/wikimetrics/database.py b/wikimetrics/database.py index fb1d92b..ff11bf7 100644 --- a/wikimetrics/database.py +++ b/wikimetrics/database.py @@ -171,7 +171,9 @@ try: json.dump(project_host_map, open(cache_name, 'w')) except: - pass # no rights to write the file, it's OK + print('No rights to write project host map cache {0}'.format( + os.path.abspath(cache_name) + )) elif os.access(cache_name, os.R_OK): project_host_map = json.load(open(cache_name)) else: @@ -198,4 +200,5 @@ # raise DisconnectionError - pool will try # connecting again up to three times before raising. raise exc.DisconnectionError() - cursor.close() + finally: + cursor.close() diff --git a/wikimetrics/models/cohort.py b/wikimetrics/models/cohort.py index e94e61c..709ea35 100644 --- a/wikimetrics/models/cohort.py +++ b/wikimetrics/models/cohort.py @@ -48,10 +48,12 @@ def __iter__(self): """ returns list of user_ids """ db_session = db.get_session() - wikiusers = self.filter_wikiuser_query( - db_session.query(WikiUser.mediawiki_userid) - ).all() - db_session.close() + try: + wikiusers = self.filter_wikiuser_query( + db_session.query(WikiUser.mediawiki_userid) + ).all() + finally: + db_session.close() return (r.mediawiki_userid for r in wikiusers) def group_by_project(self): @@ -69,10 +71,12 @@ analyzed using a single database connection """ db_session = db.get_session() - user_id_projects = self.filter_wikiuser_query( - db_session.query(WikiUser.mediawiki_userid, WikiUser.project) - ).order_by(WikiUser.project).all() - db_session.close() + try: + user_id_projects = self.filter_wikiuser_query( + db_session.query(WikiUser.mediawiki_userid, WikiUser.project) + ).order_by(WikiUser.project).all() + finally: + db_session.close() # TODO: push this logic into sqlalchemy. The solution # includes subquery(), but I can't seem to get anything working groups = itertools.groupby(user_id_projects, key=itemgetter(1)) diff --git a/wikimetrics/models/report_nodes/report.py b/wikimetrics/models/report_nodes/report.py index d0d6972..67235d4 100644 --- a/wikimetrics/models/report_nodes/report.py +++ b/wikimetrics/models/report_nodes/report.py @@ -176,11 +176,13 @@ self.result_key = str(uuid4()) db_session = db.get_session() - pj = db_session.query(PersistentReport).get(self.persistent_id) - pj.result_key = self.result_key - db_session.add(pj) - db_session.commit() - db_session.close() + try: + pj = db_session.query(PersistentReport).get(self.persistent_id) + pj.result_key = self.result_key + db_session.add(pj) + db_session.commit() + finally: + db_session.close() merged = {self.result_key: results} for child_result in child_results: diff --git a/wikimetrics/models/report_nodes/run_report.py b/wikimetrics/models/report_nodes/run_report.py index d25f4e2..f38759b 100644 --- a/wikimetrics/models/report_nodes/run_report.py +++ b/wikimetrics/models/report_nodes/run_report.py @@ -44,9 +44,11 @@ # get cohort cohort_dict = cohort_metric_dict['cohort'] - db_session = db.get_session() - cohort = Cohort.get_safely(db_session, self.user_id, by_id=cohort_dict['id']) - db_session.close() + session = db.get_session() + try: + cohort = Cohort.get_safely(session, self.user_id, by_id=cohort_dict['id']) + finally: + session.close() # construct metric metric_dict = cohort_metric_dict['metric'] -- To view, visit https://gerrit.wikimedia.org/r/102462 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib506c1a6cca5b13aaa957de98bfa837860cd5e4a Gerrit-PatchSet: 1 Gerrit-Project: analytics/wikimetrics Gerrit-Branch: master Gerrit-Owner: Milimetric <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
