From 513f24575493867e6fb4caa41137b6d920b0a4b3 Mon Sep 17 00:00:00 2001 From: FrantiĊĦek Zatloukal Date: Mar 24 2021 11:07:20 +0000 Subject: Merge branch 'develop' --- diff --git a/Makefile b/Makefile index 4c75d9a..8ab9e6e 100644 --- a/Makefile +++ b/Makefile @@ -55,7 +55,7 @@ update-makefile: test: $(VENV) set -e source $(VENV)/bin/activate; - TEST='true' py.test --cov-report=term-missing --cov $(MODULENAME); + pytest --cov-report=term-missing --cov $(MODULENAME); deactivate .PHONY: test-ci @@ -63,7 +63,7 @@ test: $(VENV) test-ci: $(VENV) set -e source $(VENV)/bin/activate - TEST='true' py.test --cov-report=xml --cov $(MODULENAME) + pytest --cov-report=xml --cov $(MODULENAME) deactivate .PHONY: pylint diff --git a/alembic/versions/72031671860e_added_votes_to_bug.py b/alembic/versions/72031671860e_added_votes_to_bug.py new file mode 100644 index 0000000..2f5b238 --- /dev/null +++ b/alembic/versions/72031671860e_added_votes_to_bug.py @@ -0,0 +1,27 @@ +"""Added votes to bug + +Revision ID: 72031671860e +Revises: 4eac64cdecd3 +Create Date: 2021-02-04 11:14:11.208780 + +""" + +# revision identifiers, used by Alembic. +revision = '72031671860e' +down_revision = '4eac64cdecd3' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('bug', sa.Column('votes', sa.Text(), + server_default=sa.sql.text('"{}"'), nullable=False)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('bug', 'votes') + # ### end Alembic commands ### diff --git a/blockerbugs.spec b/blockerbugs.spec index 199483a..126edcf 100644 --- a/blockerbugs.spec +++ b/blockerbugs.spec @@ -1,7 +1,7 @@ Name: blockerbugs # NOTE: if you update version, *make sure* to also update # `blockerbugs/__init__.py` and `docs/source/conf.py` -Version: 1.1.0 +Version: 1.2.0 Release: 1%{?dist} Summary: Fedora QA Blocker Tracking Application @@ -105,6 +105,18 @@ cp -v docs/_build/man/*.1 %{buildroot}/%{_mandir}/man1/ %changelog +* Wed Mar 24 2021 Frantisek Zatloukal - 1.2.0-1 +- Prioritized Bugs hotfix +- show TBD when a discussion link doesn't exist, show if user already voted +- Show discussion vote counts in web UI and IRC format +- tests: ignore a Koji deprecation warning +- tests: avoid ORM warnings +- supress warnings about SQLALCHEMY_TRACK_MODIFICATIONS +- initialize FAS only when needed +- run tests with just 'pytest' +- init: add pre-config logging +- cli: add --debug option + * Wed Jan 20 2021 Frantisek Zatloukal - 1.1.0-1 - discussions: ignore closed tickets diff --git a/blockerbugs/__init__.py b/blockerbugs/__init__.py index 0e4947a..3d7bdc5 100644 --- a/blockerbugs/__init__.py +++ b/blockerbugs/__init__.py @@ -1,38 +1,40 @@ -from flask import Flask, render_template -from flask_fas_openid import FAS -from flask_sqlalchemy import SQLAlchemy import logging import logging.handlers import os -import sys -from . import config +from flask import Flask, render_template +from flask_sqlalchemy import SQLAlchemy -from .util.login import FakeFas +from . import config # the version as used in setup.py and docs -__version__ = "1.1.0" - +__version__ = "1.2.0" # Flask App app = Flask(__name__) - -fas = None - # Is this an OpenShift deployment? openshift = os.getenv('OPENSHIFT_PROD') +# set up basic logging so that initial messages are not lost (until we +# configure logging properly) +_logging_fmt = '[%(name)-15s] %(asctime)s %(levelname)-7s %(message)s' +_logging_fmt_debug = '[%(name)-23s:%(lineno)-3d] %(asctime)s %(levelname)-7s %(message)s' +_logging_datefmt = '%Y-%m-%d %H:%M:%S' +logging.basicConfig(format=_logging_fmt_debug, datefmt=_logging_datefmt) +logging.getLogger().setLevel(logging.DEBUG) + # load default configuration if os.getenv('DEV') == 'true': + app.logger.debug('Using development config') app.config.from_object('blockerbugs.config.DevelopmentConfig') - fas = FakeFas(app) elif os.getenv('TEST') == 'true' or openshift == "0": - fas = FAS(app) + app.logger.debug('Using testing config') app.config.from_object('blockerbugs.config.TestingConfig') else: - fas = FAS(app) + app.logger.debug('Using production config') app.config.from_object('blockerbugs.config.ProductionConfig') if openshift: + app.logger.debug('Using openshift config') config.openshift_config(app.config, openshift) # load real configuration values to override defaults @@ -55,67 +57,62 @@ if os.getenv('DEBUG') == 'true': # Make sure config URLs end with a slash, so that we have them all in an # expected format -def end_with_slash(url): - if not url.endswith('/'): - return url + '/' - else: - return url - for key in ['BUGZILLA_URL', 'KOJI_URL', 'BODHI_URL', 'FAS_BASE_URL', - 'PAGURE_URL', 'PAGURE_API']: - app.config[key] = end_with_slash(app.config[key]) - -# "Hotfix" for proxy handling on current deployment, my guess is that the proxy -# server is set differently than it was, but what do I know... -if app.config["BEHIND_PROXY"]: - from werkzeug.contrib.fixers import ProxyFix - app.wsgi_app = ProxyFix(app.wsgi_app, num_proxies=1) - -# setup logging -fmt = '[%(filename)s:%(lineno)d] ' if app.debug else '%(module)-12s ' -fmt += '%(asctime)s %(levelname)-7s %(message)s' -datefmt = '%Y-%m-%d %H:%M:%S' -loglevel = logging.DEBUG if app.debug else logging.INFO -formatter = logging.Formatter(fmt=fmt, datefmt=datefmt) + 'PAGURE_URL', 'PAGURE_API']: + if not app.config[key].endswith('/'): + app.config[key] = app.config[key] + '/' def setup_logging(): - root_logger = logging.getLogger('') + formatter = logging.Formatter( + fmt=_logging_fmt_debug if app.debug else _logging_fmt, + datefmt=_logging_datefmt) + loglevel = logging.DEBUG if app.debug else logging.INFO + + root_logger = logging.getLogger() root_logger.setLevel(logging.DEBUG) root_logger.handlers.clear() app.logger.handlers.clear() if app.config['STREAM_LOGGING']: - app.logger.debug("doing stream logging") stream_handler = logging.StreamHandler() stream_handler.setLevel(loglevel) stream_handler.setFormatter(formatter) root_logger.addHandler(stream_handler) + app.logger.debug("doing stream logging") if app.config['SYSLOG_LOGGING']: - app.logger.debug("doing syslog logging") syslog_handler = logging.handlers.SysLogHandler( address='/dev/log', facility=logging.handlers.SysLogHandler.LOG_LOCAL4) syslog_handler.setLevel(loglevel) syslog_handler.setFormatter(formatter) root_logger.addHandler(syslog_handler) + app.logger.debug("doing syslog logging") if app.config['FILE_LOGGING'] and app.config['LOGFILE']: - app.logger.debug("doing file logging to %s" % app.config['LOGFILE']) - file_handler = logging.handlers.RotatingFileHandler(app.config['LOGFILE'], maxBytes=500000, backupCount=5) + file_handler = logging.handlers.RotatingFileHandler( + app.config['LOGFILE'], maxBytes=500000, backupCount=5) file_handler.setLevel(loglevel) file_handler.setFormatter(formatter) root_logger.addHandler(file_handler) + app.logger.debug("doing file logging to %s" % app.config['LOGFILE']) + setup_logging() +# database if app.config['SHOW_DB_URI']: app.logger.debug('using DBURI: %s' % app.config['SQLALCHEMY_DATABASE_URI']) - -# database db = SQLAlchemy(app) +# "Hotfix" for proxy handling on current deployment, my guess is that the proxy +# server is set differently than it was, but what do I know... +if app.config["BEHIND_PROXY"]: + from werkzeug.contrib.fixers import ProxyFix + app.wsgi_app = ProxyFix(app.wsgi_app, num_proxies=1) + +# === Flask views and stuff === @app.template_filter('tagify') def tagify(value): return value.replace(' ', '') diff --git a/blockerbugs/cli.py b/blockerbugs/cli.py index c1655c9..9b8236e 100644 --- a/blockerbugs/cli.py +++ b/blockerbugs/cli.py @@ -3,7 +3,7 @@ import os from argparse import ArgumentParser import secrets -from blockerbugs import db, app +from blockerbugs import db, app, setup_logging from blockerbugs.models.milestone import Milestone from blockerbugs.models.release import Release from blockerbugs.util.bug_sync import BugSync @@ -233,6 +233,9 @@ def close_inactive_discussions(args): def main(): parser = ArgumentParser() + parser.add_argument('--debug', action='store_true', default=False, + help='Enable debug logs') + subparsers = parser.add_subparsers(dest='command', metavar="") init_db_parser = subparsers.add_parser('init_db', help='Initialize DB') @@ -328,6 +331,11 @@ def main(): parser.print_help() sys.exit(1) + if args.debug: + app.config['DEBUG'] = True + # re-initialize logging with changed params + setup_logging() + # run the requested command args.func(args) diff --git a/blockerbugs/config.py b/blockerbugs/config.py index 7508923..7170b3e 100644 --- a/blockerbugs/config.py +++ b/blockerbugs/config.py @@ -29,6 +29,7 @@ class Config(object): DEBUG = True SECRET_KEY = None # REPLACE THIS WITH A RANDOM STRING SQLALCHEMY_DATABASE_URI = 'sqlite://' + SQLALCHEMY_TRACK_MODIFICATIONS = False BUGZILLA_URL = 'https://partner-bugzilla.redhat.com/' BUGZILLA_XMLRPC = BUGZILLA_URL + 'xmlrpc.cgi' KOJI_URL = 'http://koji.stg.fedoraproject.org/' @@ -96,6 +97,7 @@ class DevelopmentConfig(Config): class TestingConfig(Config): TESTING = True + SHOW_DB_URI = True def openshift_config(config_object, openshift_production): diff --git a/blockerbugs/controllers/main.py b/blockerbugs/controllers/main.py index 7270ced..8aea80e 100644 --- a/blockerbugs/controllers/main.py +++ b/blockerbugs/controllers/main.py @@ -25,9 +25,12 @@ import datetime from sqlalchemy import func, desc, or_, and_ import bugzilla from flask_fas_openid import fas_login_required +import json +import itertools from blockerbugs import app, db, __version__ from blockerbugs.util.bz_interface import BlockerProposal, BZInterfaceError +from blockerbugs.util import pagure_bot from blockerbugs.models.bug import Bug from blockerbugs.models.milestone import Milestone from blockerbugs.models.update import Update @@ -178,6 +181,100 @@ def get_milestone_info(milestone): } +def bugz_to_votes(bugz): + """Returns votes for each bug in bugz in format: + { + bugid: { + tracker_name: { + "-1": ["nick1", "nick2"], + "0": [], + "+1": ["nick3"], + } + } + } + """ + # ignore milestone key and flatten + all_bugz = itertools.chain(*bugz.values()) + votes = {} + for bug in all_bugz: + votes[bug.bugid] = json.loads(bug.votes) + return votes + + +def irc_voting_info(bugz): + """Returns voting summary in IRC format for each bug in bugz: + { + bugid: "#info Ticket vote: ...\n#info Ticket vote: ..." + } + """ + voting_info = {} + all_votes = bugz_to_votes(bugz) + for bugid, votes in all_votes.items(): + info = "" + for tracker, vote in votes.items(): + # if no one voted +1 or -1, don't show the info at all + if not (vote['-1'] or vote['+1']): + continue + summary = f"(+{len(vote['+1'])},{len(vote['0'])},-{len(vote['-1'])})" + pros = [f"+{person_vote}" for person_vote in vote['+1']] + neutrals = [f"{person_vote}" for person_vote in vote['0']] + cons = [f"-{person_vote}" for person_vote in vote['-1']] + people = ", ".join(pros + neutrals + cons) + info += f"#info Ticket vote: {pagure_bot.NICER[tracker]} {summary} ({people})\n" + voting_info[bugid] = info.strip() + + return voting_info + + +def vote_count_tuple(votes, tracker_name): + """Returns voting tuple for given tracker_name in format: + (6, 0, 3) or None + """ + vote = votes.get(tracker_name) + if not vote: + return None + + return (len(vote['+1']), len(vote['0']), len(vote['-1'])) + + +def user_voted(bug_votes): + if not g.fas_user: + return False + + voters = set() + for tracker_votes in bug_votes.values(): + voters.update(itertools.chain(*tracker_votes.values())) + + return g.fas_user.username in voters + + +def web_voting_info(bugz, milestone): + """Returns voting tuple for each bug and each section in web UI, + a dict in dict structure in format: + { + bugid: { + 'Prioritized Bugs': None, + 'Proposed Blockers': (6, 0, 3), + ... + } + } + """ + voting_info = {} + all_votes = bugz_to_votes(bugz) + for bugid, votes in all_votes.items(): + voting_info[bugid] = { + 'Proposed Blockers': vote_count_tuple(votes, f'{milestone}blocker'), + 'Accepted Blockers': vote_count_tuple(votes, f'{milestone}blocker'), + 'Proposed Freeze Exceptions': vote_count_tuple(votes, f'{milestone}freezeexception'), + 'Accepted Freeze Exceptions': vote_count_tuple(votes, f'{milestone}freezeexception'), + 'Accepted 0-day Blockers': vote_count_tuple(votes, '0day'), + 'Accepted Previous Release Blockers': vote_count_tuple(votes, 'previousrelease'), + 'Prioritized Bugs': None, # no discussion for prioritized bugs + 'user_voted': user_voted(votes), + } + return voting_info + + @main.route('/') def index(): if app.debug: @@ -205,13 +302,15 @@ def display_buglist(num, release_name): bugz = get_milestone_bugs(milestone) recent_bugs = get_recent_modifications(milestone.id) whiteboard_change = get_recent_whiteboard_change(milestone.id) + vote_info = web_voting_info(bugz, milestone.version) return render_template('blocker_list.html', buglists=bugz, recent=recent_bugs, wb_change=whiteboard_change, info=release_info, title="Fedora %s %s Blocker Bugs" % ( - release_info['number'], release_info['phase'])) + release_info['number'], release_info['phase']), + vote_info=vote_info) @main.route('/bug//updates') @@ -236,8 +335,9 @@ def display_irc_blockers(num, release_name): abort(404) bugz = get_milestone_bugs(milestone) milestone_info = get_milestone_info(milestone) - - response = make_response(render_template('irc_format.txt', buglists=bugz, info=milestone_info)) + vote_info = irc_voting_info(bugz) + response = make_response(render_template('irc_format.txt', buglists=bugz, info=milestone_info, + vote_info=vote_info)) response.mimetype = 'text/plain' return response @@ -258,6 +358,7 @@ def display_release_updates(num, release_name): updates=updates, title="Fedora %s %s Blocker Bug Updates" % (milestone_info['number'], milestone_info['phase'])) + @main.route('/milestone///requests') def display_release_requests(num, release_name): release = Release.query.filter_by(number=num).first() @@ -274,6 +375,7 @@ def display_release_requests(num, release_name): response.mimetype = 'text/plain' return response + @main.route('/milestone///need_testing') def display_updates_need_testing(num, milestone_name): release = Release.query.filter_by(number=num).first() diff --git a/blockerbugs/controllers/users.py b/blockerbugs/controllers/users.py index a3bd56d..e8ac042 100644 --- a/blockerbugs/controllers/users.py +++ b/blockerbugs/controllers/users.py @@ -23,10 +23,13 @@ # Liberally borrows from the sample app in Flask-FAS maintained by Ian Weller from flask import redirect, url_for, request, g, abort +from flask_fas_openid import fas_login_required -from blockerbugs import app, fas +from blockerbugs import app +import blockerbugs.util.login + +fas = blockerbugs.util.login.getFAS() -from flask_fas_openid import fas_login_required @app.route('/auth_login') def auth_login(): @@ -49,6 +52,7 @@ def auth_logout(): app.logger.debug('No FAS user to logout') return redirect(url_for('main.index')) + @fas_login_required def check_admin_rights(): if app.config['FAS_ADMIN_GROUP'] in g.fas_user.groups: diff --git a/blockerbugs/models/bug.py b/blockerbugs/models/bug.py index 9f4622a..5ae01d9 100644 --- a/blockerbugs/models/bug.py +++ b/blockerbugs/models/bug.py @@ -54,6 +54,7 @@ class Bug(db.Model): backref=db.backref('bugs', lazy='dynamic')) #backref='bugs') discussion_link = db.Column(db.String, unique=False) + votes = db.Column(db.Text) def __init__(self, bugid, url, summary, status, component, milestone, active, needinfo, needinfo_requestee, @@ -79,6 +80,17 @@ class Bug(db.Model): self.prioritized = False self.last_whiteboard_change = last_whiteboard_change self.last_bug_sync = last_bug_sync + # self.votes is JSON representation of dict of all BugVoteTrackers + # and corresponding output of BugVoteTracker.enumerate_votes() + # with comment ids removed: + # { + # tracker_name: { + # "-1": ["nick1", "nick2"], + # "0": [], + # "+1": ["nick3"], + # } + # } + self.votes = "{}" def __repr__(self): return '' % (self.bugid, self.summary) diff --git a/blockerbugs/templates/blocker_list.html b/blockerbugs/templates/blocker_list.html index 7e4b9b7..97a5a6d 100644 --- a/blockerbugs/templates/blocker_list.html +++ b/blockerbugs/templates/blocker_list.html @@ -77,7 +77,29 @@ text: '{{ 'Discuss' if buglist.startswith('Accepted') else 'Vote!' }} + + {% if vote_info[bug.bugid][buglist] %} + +{{ vote_info[bug.bugid][buglist][0] }}, + {{ vote_info[bug.bugid][buglist][1] }}, + -{{ vote_info[bug.bugid][buglist][2] }} +
+ {% endif %} + {% if bug.discussion_link %} + + {% if buglist.startswith('Accepted') %} + Discuss + {% elif vote_info[bug.bugid]['user_voted'] %} + Voted + {% else %} + Vote! + {% endif %} + + {% elif buglist.startswith('Prioritized') %} + + {% else %} + TBD + {% endif %} + `_ and can be run from the root project directory with:: - TEST='true' py.test testing/ - -The ``TEST='true'`` part is rather important as some of the tests will wipe the -database and fill it with less useful data. The ``TEST`` configuration forces -the use of an in-memory SQLite3 database. + pytest diff --git a/pytest.ini b/pytest.ini index 4234cd6..8932ab5 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,3 +2,6 @@ minversion = 2.0 python_functions=test should python_files=test_* testfunc_* +testpaths = testing +filterwarnings = + ignore:the imp module is deprecated in favour of importlib:DeprecationWarning:koji diff --git a/testing/conftest.py b/testing/conftest.py index 7655fe1..ba6e919 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -1,5 +1,5 @@ # -# conftest.py - utils for py.test +# conftest.py - utils for pytest # # Copyright 2011, Red Hat, Inc. # @@ -19,38 +19,10 @@ # # Author: Tim Flink -import pytest import os -def pytest_addoption(parser): - """ - Add an option to the py.test parser to detect when the functional tests - should be detected and run - """ - - parser.addoption('--functional', action='store_true', default=True, - help='Add functional tests') - - -def pytest_ignore_collect(path, config): - """Prevents collection of any files named testfunc* to speed up non - integration tests""" - if path.fnmatch('*testfunc*'): - try: - is_functional = config.getvalue('functional') - except KeyError: - return True - - return not is_functional - - def pytest_configure(config): - """This is a bit of a hack to detect whether or not we are running inside - a test environment""" - import sys - - sys._called_from_test = True - + """This is executed before testing starts""" # make sure that the testing config is used os.environ['TEST'] = 'true' diff --git a/testing/test_discussion_sync.py b/testing/test_discussion_sync.py index efbca5f..cb45cc2 100644 --- a/testing/test_discussion_sync.py +++ b/testing/test_discussion_sync.py @@ -39,6 +39,7 @@ class TestInactiveDiscussions(object): def teardown_method(self): db.session.rollback() + db.session.close() db.drop_all() def test_closing_commenting(self, monkeypatch): diff --git a/testing/testfunc_bugmodel.py b/testing/testfunc_bugmodel.py index 04b4755..15ebcd5 100644 --- a/testing/testfunc_bugmodel.py +++ b/testing/testfunc_bugmodel.py @@ -34,6 +34,7 @@ class TestfuncBugModel(object): def teardown_method(self, method): db.session.rollback() + db.session.close() db.drop_all() def test_add_release(self): diff --git a/testing/testfunc_bugsync.py b/testing/testfunc_bugsync.py index ff38c52..fdc74f3 100644 --- a/testing/testfunc_bugsync.py +++ b/testing/testfunc_bugsync.py @@ -72,6 +72,7 @@ class TestfuncBugsync(object): @classmethod def teardown_class(cls): db.session.rollback() + db.session.close() db.drop_all() def setup_method(self, method): diff --git a/testing/testfunc_update_sync.py b/testing/testfunc_update_sync.py index a9468a1..cbd0055 100644 --- a/testing/testfunc_update_sync.py +++ b/testing/testfunc_update_sync.py @@ -111,7 +111,6 @@ class FakeRaisesBodhiInterface(FakeBodhiInterface): class TestfuncUpdateSync(object): def setup_method(self, method): - db.session.flush() db.session.rollback() db.drop_all() db.create_all() @@ -128,6 +127,11 @@ class TestfuncUpdateSync(object): db.session.commit() self.update_sync = UpdateSync(db, FakeBodhiInterface) + def teardown_method(self, method): + db.session.rollback() + db.session.close() + db.drop_all() + def test_sync_single_bug_with_one_update(self): bug1 = add_bug(2000, 'testbug1', self.test_milestone99alpha) self.update_sync.sync_updates(self.test_release99)