From 6b7980dcbf5c2e9c4fd09dba242363eeedaad2f7 Mon Sep 17 00:00:00 2001 From: Randy Barlow Date: Thu, 21 Feb 2019 14:34:59 -0500 Subject: [PATCH] Drop the updates.title column. The updates.title field used to be a space separated list of the builds found in the update, with a uniqueness constraint. There were at least two problems with it. One problem was that the list of builds was not consistently updated (#1946) and sometimes the title did not reflect the current set of builds in the update. This was more severe than it may seem, because it turns out that the CLI was using the title to set the list of builds when editing updates, instead of using the list of builds. This means that the CLI could set the list of builds back to a former set, rather than leaving it at the set it was currently at. Note that the CLI does not currently support editing the set of builds on an update (#3037), so it is especially surprising when this happens. The second problem is that large updates with many builds end up generating a very long title and PostgreSQL raises an error because it refuses to index strings beyond a certain length. It was found, for example, that release engineering could not create an update with 300 builds in it. Since we plan to use Bodhi to work with side tags as part of gating Fedora Rawhide, it will be important for users to be able to create updates with large numbers of builds. The title was also not a particularly useful field. It was possible to use it as a key to access an update, but that was a poor use for it since the title changes when the list of builds change, and because the order of the list of builds was important. This led to unpredictable and unstable URLs for updates, and Bodhi already had an update alias field which is stable and is better suited for that purpose. This commit drops the field from the database and removes it from being used to reference updates in the API. It preserves it in API responses, however, but now generates it dynamically so that the title is always an accurate list of the builds included in the update. fixes #186 fixes #1542 fixes #1946 Signed-off-by: Randy Barlow --- bodhi/client/__init__.py | 35 ++-- bodhi/client/bindings.py | 31 ++- bodhi/server/consumers/masher.py | 22 +-- bodhi/server/consumers/updates.py | 9 +- bodhi/server/mail.py | 12 +- bodhi/server/metadata.py | 4 +- ...19b942c_remove_the_updates_title_column.py | 53 ++++++ bodhi/server/models.py | 101 +++++----- bodhi/server/push.py | 8 +- bodhi/server/renderers.py | 4 +- bodhi/server/scripts/approve_testing.py | 6 +- bodhi/server/scripts/untag_branched.py | 3 +- bodhi/server/services/builds.py | 6 +- bodhi/server/services/releases.py | 6 +- bodhi/server/services/updates.py | 17 +- bodhi/server/services/user.py | 6 +- bodhi/server/static/js/search.js | 2 - bodhi/server/static/js/update_form.js | 2 +- bodhi/server/templates/new_update.html | 11 +- bodhi/server/templates/update.html | 17 +- bodhi/server/util.py | 32 ++-- bodhi/server/validators.py | 13 +- bodhi/tests/client/__init__.py | 2 +- bodhi/tests/client/test___init__.py | 180 ++++-------------- bodhi/tests/client/test_bindings.py | 32 +--- docs/user/release_notes.rst | 1 + 26 files changed, 229 insertions(+), 386 deletions(-) create mode 100644 bodhi/server/migrations/versions/58b7919b942c_remove_the_updates_title_column.py diff --git a/bodhi/client/__init__.py b/bodhi/client/__init__.py index 4a36e9310e..6ccabc53c4 100644 --- a/bodhi/client/__init__.py +++ b/bodhi/client/__init__.py @@ -421,7 +421,7 @@ def _validate_edit_update(ctx, param, value): """ Validate the update argument given to the updates edit command. - The update argument can only be update id or update title + The update argument can only be an update id. Args: param (basestring): The name of the parameter being validated. Unused. @@ -435,7 +435,7 @@ def _validate_edit_update(ctx, param, value): or re.search(bindings.UPDATE_TITLE_RE, value): return value else: - raise click.BadParameter("Please provide an Update ID or an Update Title") + raise click.BadParameter("Please provide an Update ID") @updates.command() @@ -452,11 +452,11 @@ def edit(user, password, url, debug, openid_api, **kwargs): """ Edit an existing update. - UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850) + UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850) """ # Developer Docs """ - The update argument can be an update id or the update title. + The update argument must be an update id. Args: user (unicode): The username to authenticate as. @@ -476,16 +476,9 @@ def edit(user, password, url, debug, openid_api, **kwargs): if re.search(bindings.UPDATE_ID_RE, kwargs['update']): query_param = {'updateid': kwargs['update']} resp = client.query(**query_param) - title = resp['updates'][0]['title'] else: - # _validate_edit_update() has already ensured that we either got an update ID or an NVR, - # so we can assume here that we have an NVR. - query_param = {'like': kwargs['update']} - resp = client.query(**query_param) - title = kwargs['update'] + raise click.BadParameter(f"{kwargs['update']} is not a valid update id.") del(kwargs['update']) - kwargs['builds'] = title - kwargs['edited'] = title # Convert list of 'Bug' instances in DB to comma separated bug_ids for parsing. former_update = resp['updates'][0].copy() @@ -493,6 +486,9 @@ def edit(user, password, url, debug, openid_api, **kwargs): kwargs['bugs'] = ",".join([str(bug['bug_id']) for bug in former_update['bugs']]) former_update.pop('bugs', None) + kwargs['builds'] = [b['nvr'] for b in former_update['builds']] + kwargs['edited'] = former_update['alias'] + # Replace empty fields with former values from database. for field in kwargs: if kwargs[field] in (None, '') and field in former_update: @@ -508,7 +504,6 @@ def edit(user, password, url, debug, openid_api, **kwargs): @updates.command() @click.option('--updateid', help='Query by update ID (eg: FEDORA-2015-0001)') -@click.option('--title', help='Query by title') @click.option('--alias', help='Query by alias') @click.option('--approved-since', help='Approved after a specific timestamp') @click.option('--approved-before', help='Approved before a specific timestamp') @@ -594,7 +589,7 @@ def request(update, state, user, password, url, openid_api, **kwargs): """ Change an update's request status. - UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850) + UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850) STATE: The state you wish to change the update's request to. Valid options are testing, stable, obsolete, unpush, and revoke. @@ -642,7 +637,7 @@ def comment(update, text, karma, user, password, url, openid_api, **kwargs): """ Comment on an update. - UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850) + UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850) TEXT: the comment to be added to the update """ @@ -720,7 +715,7 @@ def download(url, **kwargs): # *think* that should ever be possible for these opts. for update in resp.updates: - click.echo("Downloading packages from {0}".format(update['title'])) + click.echo("Downloading packages from {0}".format(update['alias'])) for build in update['builds']: # subprocess is icky, but koji module doesn't # expose this in any usable way, and we don't want @@ -784,13 +779,13 @@ def waive(update, show, test, comment, url, openid_api, **kwargs): """ Show or waive unsatified requirements (ie: missing or failing tests) on an existing update. - UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850) + UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850) COMMENT: A comment explaining why the requirements were waived (mandatory with --test) """ # Developer Docs """ - The update argument can be an update id or the update title. + The update argument must be an update id.. Args: update (unicode): The update who unsatisfied requirements wish to waive. @@ -1017,7 +1012,7 @@ def print_resp(resp, client, verbose=False, override_hint=True): resp.total, len(resp.updates))) elif resp.get('update'): click.echo(client.update_str(resp['update'])) - elif 'title' in resp: + elif resp.get('alias'): click.echo(client.update_str(resp)) elif 'overrides' in resp: if len(resp.overrides) == 1: @@ -1035,7 +1030,7 @@ def print_resp(resp, client, verbose=False, override_hint=True): if override_hint: _print_override_koji_hint(resp, client) elif 'comment' in resp: - click.echo('The following comment was added to %s' % resp.comment['update'].title) + click.echo('The following comment was added to %s' % resp.comment['update'].alias) click.echo(resp.comment.text) elif 'compose' in resp: click.echo(client.compose_str(resp['compose'], minimal=False)) diff --git a/bodhi/client/bindings.py b/bodhi/client/bindings.py index 9316543a60..0943fa9a0e 100644 --- a/bodhi/client/bindings.py +++ b/bodhi/client/bindings.py @@ -226,7 +226,7 @@ def save(self, **kwargs): Save an update. This entails either creating a new update, or editing an existing one. - To edit an existing update, you must specify the update title in + To edit an existing update, you must specify the update alias in the ``edited`` keyword argument. Args: @@ -250,7 +250,7 @@ def save(self, **kwargs): stable_karma (int): The upper threshold for marking an update as ``stable``. unstable_karma (int): The lower threshold for unpushing an update. - edited (basestring): The update title of the existing update that we are + edited (basestring): The update alias of the existing update that we are editing. severity (basestring): The severity of this update (``urgent``, ``high``, ``medium``, ``low``). @@ -277,7 +277,7 @@ def request(self, update, request): Request an update state change. Args: - update (basestring): The title of the update. + update (basestring): The alias of the update. request (basestring): The request (``testing``, ``stable``, ``obsolete``, ``unpush``, ``revoke``). Returns: @@ -303,7 +303,7 @@ def waive(self, update, comment, tests=None): Waive unsatisfied requirements on an update. Args: - update (basestring): The title of the update. + update (basestring): The alias of the update. comment (basestring): A comment explaining the waiver. tests (tuple(basestring) or None): The list of unsatisfied requirements to waive. If not specified, all unsatisfied requirements of this @@ -330,7 +330,6 @@ def query(self, **kwargs): Query bodhi for a list of updates. Args: - title (basestring): The update title. alias (basestring): The update alias. updateid (basestring): The update ID (eg: FEDORA-2015-0001). content_type (basestring): A content type (rpm, module) to limit the query to. @@ -376,9 +375,6 @@ def query(self, **kwargs): Returns: munch.Munch: The response from Bodhi describing the query results. """ - if 'title' in kwargs: - kwargs['like'] = kwargs['title'] - del kwargs['title'] # bodhi1 compat if 'limit' in kwargs: kwargs['rows_per_page'] = kwargs['limit'] @@ -416,7 +412,7 @@ def get_test_status(self, update): Query bodhi for the test status of the specified update.. Args: - update (basestring): The title or identifier of the update to + update (basestring): The alias of the update to retrieve the test status of. Returns: munch.Munch: The response from Bodhi describing the query results. @@ -429,7 +425,7 @@ def comment(self, update, comment, karma=0, email=None): Add a comment to an update. Args: - update (basestring): The title of the update comment on. + update (basestring): The alias of the update comment on. comment (basestring): The text of the comment to add to the update. karma (int): The amount of karma to leave. May be -1, 0, or 1. Defaults to 0. email (basestring or None): Email address for an anonymous user. If an email address is @@ -798,16 +794,15 @@ def update_str(self, update, minimal=False): update_lines = ['{:=^80}\n'.format('=')] update_lines += [ line + '\n' for line in textwrap.wrap( - update['title'].replace(',', ', '), + update['title'], width=80, initial_indent=' ' * 5, subsequent_indent=' ' * 5) ] update_lines.append('{:=^80}\n'.format('=')) - if update['alias']: - update_lines.append( - line_formatter.format('Update ID', update['alias'])) + update_lines.append( + line_formatter.format('Update ID', update['alias'])) update_lines += [ line_formatter.format('Content Type', update['content_type']), @@ -902,12 +897,8 @@ def update_str(self, update, minimal=False): comments_lines[1:]) ] - if update['alias']: - update_lines.append( - '\n {0}updates/{1}\n'.format(self.base_url, update['alias'])) - else: - update_lines.append( - '\n {0}updates/{1}\n'.format(self.base_url, update['title'])) + update_lines.append( + '\n {0}updates/{1}\n'.format(self.base_url, update['alias'])) return ''.join(update_lines) diff --git a/bodhi/server/consumers/masher.py b/bodhi/server/consumers/masher.py index 4b057d0ed0..be07090575 100644 --- a/bodhi/server/consumers/masher.py +++ b/bodhi/server/consumers/masher.py @@ -1,4 +1,4 @@ -# Copyright © 2007-2018 Red Hat, Inc. and others. +# Copyright © 2007-2019 Red Hat, Inc. and others. # # This file is part of Bodhi. # @@ -211,14 +211,6 @@ def _get_composes(self, msg): with self.db_factory() as db: if 'api_version' in msg and msg['api_version'] == 2: composes = [Compose.from_dict(db, c) for c in msg['composes']] - elif 'updates' in msg: - updates = [db.query(Update).filter(Update.title == t).one() for t in msg['updates']] - composes = Compose.from_updates(updates) - for c in composes: - db.add(c) - # This flush is necessary so the compose finds its updates, which gives it a - # content_type when it is serialized later. - db.flush() else: raise ValueError('Unable to process fedmsg: {}'.format(msg)) @@ -383,7 +375,7 @@ def work(self): notifications.publish( topic="mashtask.mashing", msg=dict(repo=self.id, - updates=[u.title for u in self.compose.updates], + updates=[' '.join([b.nvr for b in u.builds]) for u in self.compose.updates], agent=self.agent, ctype=self.ctype.value), force=True, @@ -469,7 +461,7 @@ def perform_gating(self): for update in self.compose.updates: result, reason = update.check_requirements(self.db, config) if not result: - self.log.warning("%s failed gating: %s" % (update.title, reason)) + self.log.warning("%s failed gating: %s" % (update.alias, reason)) self.eject_from_mash(update, reason) # We may have removed some updates from this compose above, and do we don't want future # reads on self.compose.updates to see those, so let's mark that attribute expired so @@ -486,7 +478,7 @@ def eject_from_mash(self, update, reason): comment on the update, in a log message, and in a fedmsg. """ update.locked = False - text = '%s ejected from the push because %r' % (update.title, reason) + text = '%s ejected from the push because %r' % (update.alias, reason) log.warning(text) update.comment(self.db, text, author=u'bodhi') # Remove the pending tag as well @@ -721,7 +713,7 @@ def modify_bugs(self): """Mark bugs on each Update as modified.""" self.log.info('Updating bugs') for update in self.compose.updates: - self.log.debug('Modifying bugs for %s', update.title) + self.log.debug('Modifying bugs for %s', update.alias) update.modify_bugs() @checkpoint @@ -766,7 +758,7 @@ def send_testing_digest(self): maildata += u' %3i %s %s\n' % ( update.days_in_testing, update.abs_url(), - update.title) + update.alias) maildata += '\n\n' critpath_updates = self.get_unapproved_critpath_updates(prefix) @@ -776,7 +768,7 @@ def send_testing_digest(self): maildata += u' %3i %s %s\n' % ( update.days_in_testing, update.abs_url(), - update.title) + update.alias) maildata += '\n\n' maildata += testhead % prefix diff --git a/bodhi/server/consumers/updates.py b/bodhi/server/consumers/updates.py index ff85771289..d88bc064cc 100644 --- a/bodhi/server/consumers/updates.py +++ b/bodhi/server/consumers/updates.py @@ -1,4 +1,4 @@ -# Copyright 2015-2018 Red Hat Inc., and others. +# Copyright 2015-2019 Red Hat Inc., and others. # # This file is part of Bodhi. # @@ -111,11 +111,6 @@ def consume(self, message): # https://github.com/fedora-infra/bodhi/issues/458 time.sleep(1) - if not alias: - log.error("Update Handler got update with no " - "alias %s." % pprint.pformat(msg)) - return - with self.db_factory() as session: update = Update.get(alias) if not update: @@ -202,7 +197,7 @@ def work_on_bugs(self, session, update, bugs): log.info("Commenting on %r" % bug.bug_id) comment = config['initial_bug_msg'] % ( - update.title, update.release.long_name, update.abs_url()) + update.alias, update.release.long_name, update.abs_url()) log.info("Modifying %r" % bug.bug_id) bug.modified(update, comment) diff --git a/bodhi/server/mail.py b/bodhi/server/mail.py index d595f64966..19ce291a20 100644 --- a/bodhi/server/mail.py +++ b/bodhi/server/mail.py @@ -50,7 +50,7 @@ %(email)s has deleted the %(package)s update for %(release)s\n\n%(updatestr)s """, 'fields': lambda agent, x: { - 'package': x.title, + 'package': x.alias, 'email': agent, 'release': '%s %s' % (x.release.long_name, x.status), 'updatestr': str(x) @@ -62,7 +62,7 @@ %(email)s has edited the %(package)s update for %(release)s\n\n%(updatestr)s """, 'fields': lambda agent, x: { - 'package': x.title, + 'package': x.alias, 'email': agent, 'release': '%s %s' % (x.release.long_name, x.status), 'updatestr': str(x) @@ -74,7 +74,7 @@ %(package)s has been successfully pushed for %(release)s.\n\n%(updatestr)s """, 'fields': lambda agent, x: { - 'package': x.title, + 'package': x.alias, 'release': '%s %s' % (x.release.long_name, x.status), 'updatestr': str(x) } @@ -187,7 +187,7 @@ %(updatestr)s """, 'fields': lambda agent, x: { - 'package': x.title, + 'package': x.alias, 'comment': x.comments[-1], 'updatestr': str(x) } @@ -214,7 +214,7 @@ %(updatestr)s """, 'fields': lambda agent, x: { - 'package': x.title, + 'package': x.alias, 'stablekarma': x.stable_karma, 'updatestr': str(x) } @@ -232,7 +232,7 @@ https://admin.fedoraproject.org/updates/approve/%(package)s """, 'fields': lambda agent, x: { - 'package': x.title, + 'package': x.alias, 'submitter': agent, 'updatestr': str(x) } diff --git a/bodhi/server/metadata.py b/bodhi/server/metadata.py index aceb93f8a9..a83d832aae 100644 --- a/bodhi/server/metadata.py +++ b/bodhi/server/metadata.py @@ -142,8 +142,6 @@ def __init__(self, release, request, db, mashdir, close_shelf=True): self.uinfo = cr.UpdateInfo() for update in self.updates: - if not update.alias: - update.assign_alias() self.add_update(update) if close_shelf: @@ -205,7 +203,7 @@ def add_update(self, update): rec.status = update.status.value rec.type = update.type.value rec.id = to_bytes(update.alias) - rec.title = to_bytes(update.title) + rec.title = to_bytes(update.beautify_title) rec.severity = util.severity_updateinfo_str(update.severity.value) rec.summary = to_bytes('%s %s update' % (update.get_title(), update.type.value)) diff --git a/bodhi/server/migrations/versions/58b7919b942c_remove_the_updates_title_column.py b/bodhi/server/migrations/versions/58b7919b942c_remove_the_updates_title_column.py new file mode 100644 index 0000000000..b092e3487a --- /dev/null +++ b/bodhi/server/migrations/versions/58b7919b942c_remove_the_updates_title_column.py @@ -0,0 +1,53 @@ +# Copyright (c) 2019 Red Hat, Inc. +# +# This file is part of Bodhi. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +""" +Remove the updates.title column. + +Revision ID: 58b7919b942c +Revises: aae0d29d49b7 +Create Date: 2019-02-20 17:13:01.260748 +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '58b7919b942c' +down_revision = 'eec610d7ab3a' + + +def upgrade(): + """Drop the updates.title column.""" + op.alter_column('updates', 'alias', existing_type=sa.BOOLEAN(), nullable=False) + op.drop_index('ix_updates_title', table_name='updates') + op.drop_column('updates', 'title') + + +def downgrade(): + """Bring back the updates.title column and try to guess what it should be set to.""" + op.add_column('updates', sa.Column('title', sa.TEXT(), autoincrement=False, nullable=True)) + # Set the title back to something similar to what it might have been. This isn't guaranteed to + # be the title it was before, because we can't know what order the nvrs appeared in, but it will + # set it to the expected format and expected set of NVRs. Single build updates should at least + # get their old title back. + op.execute( + ("UPDATE updates SET title=(" + "SELECT string_agg(nvr, ' ') as title FROM (" + "SELECT builds.nvr FROM builds WHERE update_id=updates.id ORDER BY nvr) as nvr)")) + op.create_index('ix_updates_title', 'updates', ['title'], unique=True) + op.alter_column('updates', 'alias', existing_type=sa.BOOLEAN(), nullable=True) diff --git a/bodhi/server/models.py b/bodhi/server/models.py index 860cd70ca1..97d53976db 100644 --- a/bodhi/server/models.py +++ b/bodhi/server/models.py @@ -20,7 +20,6 @@ from collections import defaultdict from datetime import datetime from textwrap import wrap -from urllib.parse import quote import hashlib import json import os @@ -1555,9 +1554,6 @@ class Update(Base): This model represents an update. Attributes: - title (unicode): The update's title which uniquely identifies the update. - This is generally an ordered list of the build NVRs contained in the - update. autokarma (bool): A boolean that indicates whether or not the update will be automatically pushed when the stable_karma threshold is reached. stable_karma (int): A positive integer that indicates the amount of "good" @@ -1631,10 +1627,8 @@ class Update(Base): __tablename__ = 'updates' __exclude_columns__ = ('id', 'user_id', 'release_id') - __include_extras__ = ('meets_testing_requirements', 'url',) - __get_by__ = ('title', 'alias') - - title = Column(UnicodeText, unique=True, default=None, index=True) + __include_extras__ = ('meets_testing_requirements', 'url', 'title') + __get_by__ = ('alias',) autokarma = Column(Boolean, default=True, nullable=False) stable_karma = Column(Integer, nullable=True) @@ -1672,7 +1666,7 @@ class Update(Base): date_stable = Column(DateTime) # eg: FEDORA-EPEL-2009-12345 - alias = Column(Unicode(32), unique=True, nullable=True) + alias = Column(Unicode(32), unique=True, nullable=False) # One-to-one relationships release_id = Column(Integer, ForeignKey('releases.id'), nullable=False) @@ -1953,7 +1947,6 @@ def new(cls, request, data): db = request.db user = User.get(request.user.name) data['user'] = user - data['title'] = ' '.join([b.nvr for b in data['builds']]) caveats = [] data['critpath'] = cls.contains_critpath_component( data['builds'], data['release'].name) @@ -2031,7 +2024,7 @@ def edit(cls, request, data): db = request.db buildinfo = request.buildinfo koji = request.koji - up = db.query(Update).filter_by(title=data['edited']).first() + up = db.query(Update).filter_by(alias=data['edited']).first() del(data['edited']) caveats = [] @@ -2097,8 +2090,6 @@ def edit(cls, request, data): up.comment(db, comment, karma=0, author=u'bodhi') caveats.append({'name': 'builds', 'description': comment}) - data['title'] = ' '.join(sorted([b.nvr for b in up.builds])) - # Updates with new or removed builds always go back to testing if new_builds or removed_builds: data['request'] = UpdateRequest.testing @@ -2277,6 +2268,10 @@ def get_tags(self): """ return list(set(sum([b.get_tags() for b in self.builds], []))) + @property + def title(self): + return self.get_title() + def get_title(self, delim=' ', limit=None, after_limit='…'): u""" Return a title for the update based on the :class:`Builds ` it is associated with. @@ -2411,7 +2406,7 @@ def assign_alias(self): year = time.localtime()[0] id = hashlib.sha1(str(uuid.uuid4()).encode('utf-8')).hexdigest()[:10] alias = u'%s-%s-%s' % (prefix, year, id) - log.debug('Setting alias for %s to %s' % (self.title, alias)) + log.debug('Setting alias for %s to %s' % (self.get_title(), alias)) self.alias = alias def set_request(self, db, action, username): @@ -2436,10 +2431,10 @@ def set_request(self, db, action, username): if isinstance(action, str): action = UpdateRequest.from_string(action) if self.status and action.description == self.status.description: - log.info("%s already %s" % (self.title, action.description)) + log.info("%s already %s" % (self.alias, action.description)) return if action is self.request: - log.debug("%s has already been submitted to %s" % (self.title, + log.debug("%s has already been submitted to %s" % (self.alias, self.request.description)) return @@ -2453,11 +2448,11 @@ def set_request(self, db, action, username): self.comment(db, u'This update has been unpushed.', author=username) notifications.publish(topic=topic, msg=dict( update=self, agent=username)) - log.debug("%s has been unpushed." % self.title) + log.debug("%s has been unpushed." % self.alias) return elif action is UpdateRequest.obsolete: self.obsolete(db) - log.debug("%s has been obsoleted." % self.title) + log.debug("%s has been obsoleted." % self.alias) notifications.publish(topic=topic, msg=dict( update=self, agent=username)) return @@ -2468,7 +2463,7 @@ def set_request(self, db, action, username): and action is UpdateRequest.revoke: self.status = UpdateStatus.unpushed self.revoke() - log.debug("%s has been revoked." % self.title) + log.debug("%s has been revoked." % self.alias) notifications.publish(topic=topic, msg=dict( update=self, agent=username)) return @@ -2478,14 +2473,14 @@ def set_request(self, db, action, username): elif self.request == UpdateRequest.stable and \ self.status is UpdateStatus.testing and action is UpdateRequest.revoke: self.revoke() - log.debug("%s has been revoked." % self.title) + log.debug("%s has been revoked." % self.alias) notifications.publish(topic=topic, msg=dict( update=self, agent=username)) return elif action is UpdateRequest.revoke: self.revoke() - log.debug("%s has been revoked." % self.title) + log.debug("%s has been revoked." % self.alias) notifications.publish(topic=topic, msg=dict( update=self, agent=username)) return @@ -2524,7 +2519,7 @@ def set_request(self, db, action, username): # Check if we've met the karma requirements if (self.stable_karma not in (None, 0) and self.karma >= self.stable_karma) \ or self.critpath_approved: - log.debug('%s meets stable karma requirements' % self.title) + log.debug('%s meets stable karma requirements' % self.alias) else: # If we haven't met the stable karma requirements, check if it # has met the mandatory time-in-testing requirements @@ -2563,7 +2558,7 @@ def set_request(self, db, action, username): flash_notes = flash_notes and '. %s' % flash_notes log.debug( "%s has been submitted for %s. %s%s" % ( - self.title, action.description, notes, flash_notes)) + self.alias, action.description, notes, flash_notes)) self.comment(db, u'This update has been submitted for %s by %s. %s' % ( action.description, username, notes), author=u'bodhi') topic = u'update.request.%s' % action @@ -2626,9 +2621,9 @@ def add_tag(self, tag): Args: tag (basestring): The tag to be added to the builds. """ - log.debug('Adding tag %s to %s' % (tag, self.title)) + log.debug('Adding tag %s to %s', tag, self.get_title()) if not tag: - log.warning("Not adding builds of %s to empty tag" % self.title) + log.warning("Not adding builds of %s to empty tag", self.title) return [] # An empty iterator in place of koji multicall koji = buildsys.get_session() @@ -2650,9 +2645,9 @@ def remove_tag(self, tag, koji=None): list or None: If a koji client was provided, ``None`` is returned. Else, a list of tasks from ``koji.multiCall()`` are returned. """ - log.debug('Removing tag %s from %s' % (tag, self.title)) + log.debug('Removing tag %s from %s', tag, self.get_title()) if not tag: - log.warning("Not removing builds of %s from empty tag" % self.title) + log.warning("Not removing builds of %s from empty tag", self.get_title()) return [] # An empty iterator in place of koji multicall return_multicall = not koji @@ -2672,12 +2667,12 @@ def modify_bugs(self): """ if self.status is UpdateStatus.testing: for bug in self.bugs: - log.debug('Adding testing comment to bugs for %s', self.title) + log.debug('Adding testing comment to bugs for %s', self.alias) bug.testing(self) elif self.status is UpdateStatus.stable: if not self.close_bugs: for bug in self.bugs: - log.debug('Adding stable comment to bugs for %s', self.title) + log.debug('Adding stable comment to bugs for %s', self.alias) bug.add_comment(self) else: if self.type is UpdateType.security: @@ -2709,7 +2704,7 @@ def status_comment(self, db): def send_update_notice(self): """Send e-mail notices about this update.""" - log.debug("Sending update notice for %s" % self.title) + log.debug("Sending update notice for %s", self.alias) mailinglist = None sender = config.get('bodhi_email') if not sender: @@ -2742,10 +2737,7 @@ def get_url(self): basestring: A URL. """ path = ['updates'] - if self.alias: - path.append(self.alias) - else: - path.append(quote(self.title)) + path.append(self.alias) return os.path.join(*path) def abs_url(self, request=None): @@ -2768,10 +2760,8 @@ def __str__(self): basestring: A string representation of the update. """ val = u"%s\n%s\n%s\n" % ('=' * 80, u'\n'.join(wrap( - self.title.replace(',', ', '), width=80, initial_indent=' ' * 5, + self.alias, width=80, initial_indent=' ' * 5, subsequent_indent=' ' * 5)), '=' * 80) - if self.alias: - val += u" Update ID: %s\n" % self.alias val += u""" Release: %s Status: %s Type: %s @@ -2865,9 +2855,9 @@ def obsolete_if_unstable(self, db): if self.autokarma and self.status is UpdateStatus.pending \ and self.request is UpdateRequest.testing and self.unstable_karma not in (0, None) \ and self.karma <= self.unstable_karma: - log.info("%s has reached unstable karma thresholds" % self.title) + log.info("%s has reached unstable karma thresholds", self.alias) self.obsolete(db) - log.debug("%s has been obsoleted." % self.title) + log.debug("%s has been obsoleted.", self.alias) return def comment(self, session, text, karma=0, author=None, anonymous=False, @@ -2934,9 +2924,9 @@ def comment(self, session, text, karma=0, author=None, anonymous=False, 'description': 'Your karma standing was reversed.', }) else: - log.debug('Ignoring duplicate %d karma from %s on %s' % (karma, author, self.title)) + log.debug('Ignoring duplicate %d karma from %s on %s', karma, author, self.alias) - log.info("Updated %s karma to %d" % (self.title, self.karma)) + log.info("Updated %s karma to %d", self.alias, self.karma) if check_karma and author not in config.get('system_users'): try: @@ -3002,11 +2992,11 @@ def unpush(self, db): Raises: BodhiException: If the update isn't in testing. """ - log.debug("Unpushing %s" % self.title) + log.debug("Unpushing %s", self.alias) koji = buildsys.get_session() if self.status is UpdateStatus.unpushed: - log.debug("%s already unpushed" % self.title) + log.debug("%s already unpushed", self.alias) return if self.status is not UpdateStatus.testing: @@ -3030,7 +3020,7 @@ def revoke(self): BodhiException: If the update doesn't have a request set, or if it is not in an expected status. """ - log.debug("Revoking %s" % self.title) + log.debug("Revoking %s", self.alias) if not self.request: raise BodhiException( @@ -3059,7 +3049,7 @@ def untag(self, db): Args: db (sqlalchemy.orm.session.Session): A database session. """ - log.info("Untagging %s" % self.title) + log.info("Untagging %s", self.alias) koji = buildsys.get_session() tag_types, tag_rels = Release.get_tags(db) for build in self.builds: @@ -3083,7 +3073,7 @@ def obsolete(self, db, newer=None): newer (Update or None): If given, the update that has obsoleted this one. Defaults to ``None``. """ - log.debug("Obsoleting %s" % self.title) + log.debug("Obsoleting %s", self.alias) self.untag(db) self.status = UpdateStatus.obsolete self.request = None @@ -3221,7 +3211,7 @@ def check_karma_thresholds(self, db, agent): """ # Raise Exception if the update is locked if self.locked: - log.debug('%s locked. Ignoring karma thresholds.' % self.title) + log.debug('%s locked. Ignoring karma thresholds.', self.alias) raise LockedUpdateException # Return if the status of the update is not in testing or pending if self.status not in (UpdateStatus.testing, UpdateStatus.pending): @@ -3235,7 +3225,7 @@ def check_karma_thresholds(self, db, agent): self.comment(db, text, author=u'bodhi') elif self.stable_karma and self.karma >= self.stable_karma: if self.autokarma: - log.info("Automatically marking %s as stable" % self.title) + log.info("Automatically marking %s as stable", self.alias) self.set_request(db, UpdateRequest.stable, agent) self.date_pushed = None notifications.publish( @@ -3245,12 +3235,12 @@ def check_karma_thresholds(self, db, agent): # Add the 'testing_approval_msg_based_on_karma' message now log.info(( "%s update has reached the stable karma threshold and can be pushed to " - "stable now if the maintainer wishes") % self.title) + "stable now if the maintainer wishes"), self.alias) elif self.unstable_karma and self.karma <= self.unstable_karma: if self.status is UpdateStatus.pending and not self.autokarma: pass else: - log.info("Automatically unpushing %s" % self.title) + log.info("Automatically unpushing %s", self.alias) self.obsolete(db) notifications.publish( topic='update.karma.threshold.reach', @@ -3498,7 +3488,7 @@ def requested_tag(self): tag = self.release.candidate_tag if not tag: raise RuntimeError( - 'Unable to determine requested tag for %s.' % self.title) + f'Unable to determine requested tag for {self.alias}.') return tag def __json__(self, request=None, anonymize=False): @@ -3644,7 +3634,7 @@ def from_updates(cls, updates): work = {} for update in updates: if not update.request: - log.info('%s request was revoked', update.title) + log.info('%s request was revoked', update.alias) continue # ASSUMPTION: For now, updates can only be of a single type. ctype = None @@ -3655,8 +3645,7 @@ def from_updates(cls, updates): # This branch is not covered because the Update.validate_builds validator # catches the same assumption breakage. This check here is extra for the # time when someone adds multitype updates and forgets to update this. - raise ValueError('Builds of multiple types found in %s' - % update.title) + raise ValueError(f'Builds of multiple types found in {update.alias}') # This key is just to insert things in the same place in the "work" # dict. key = '%s-%s' % (update.release.name, update.request.value) @@ -3899,8 +3888,8 @@ def __json__(self, *args, **kwargs): else: result['author'] = u'anonymous' - # Similarly, duplicate the update's title as update_title. - result['update_title'] = result['update']['title'] + # Similarly, duplicate the update's alias as update_alias. + result['update_alias'] = result['update']['alias'] # Updates used to have a karma column which would be included in result['update']. The # column was replaced with a property, so we need to include it here for backwards diff --git a/bodhi/server/push.py b/bodhi/server/push.py index 7d45d0b6e9..d27713b00f 100644 --- a/bodhi/server/push.py +++ b/bodhi/server/push.py @@ -1,4 +1,4 @@ -# Copyright © 2007-2018 Red Hat, Inc. +# Copyright © 2007-2019 Red Hat, Inc. # # This file is part of Bodhi. # @@ -128,8 +128,8 @@ def push(username, cert_prefix, yes, **kwargs): update_sig_status(update) if not update.signed: - click.echo('Warning: %s has unsigned builds and has been skipped' % - update.title) + click.echo( + 'Warning: {update.get_title()} has unsigned builds and has been skipped') continue updates.append(update) @@ -155,7 +155,7 @@ def push(username, cert_prefix, yes, **kwargs): for compose in composes: click.echo('\n\n===== {} =====\n'.format(compose)) for update in compose.updates: - click.echo(update.title) + click.echo(update.get_title()) if composes: if yes: diff --git a/bodhi/server/renderers.py b/bodhi/server/renderers.py index 6831fa710b..f16b60038c 100644 --- a/bodhi/server/renderers.py +++ b/bodhi/server/renderers.py @@ -1,4 +1,4 @@ -# Copyright © 2014-2017 Red Hat, Inc. +# Copyright © 2014-2019 Red Hat, Inc. # # This file is part of Bodhi. # @@ -87,7 +87,7 @@ def link_dict(obj): getters = { 'updates': { - 'title': operator.itemgetter('title'), + 'title': operator.itemgetter('alias'), 'link': linker('update', 'id', 'title'), 'description': operator.itemgetter('notes'), 'pubdate': lambda obj: utc.localize(obj['date_submitted']), diff --git a/bodhi/server/scripts/approve_testing.py b/bodhi/server/scripts/approve_testing.py index 472a15e40f..efeccac449 100644 --- a/bodhi/server/scripts/approve_testing.py +++ b/bodhi/server/scripts/approve_testing.py @@ -1,4 +1,4 @@ -# Copyright © 2013-2017 Red Hat, Inc. and others. +# Copyright © 2013-2019 Red Hat, Inc. and others. # # This file is part of Bodhi. # @@ -87,7 +87,7 @@ def main(argv=sys.argv): # Approval message when testing based on karma threshold if update.stable_karma not in (0, None) and update.karma >= update.stable_karma \ and not update.autokarma and update.meets_testing_requirements: - print('%s now reaches stable karma threshold' % update.title) + print(f'{update.alias} now reaches stable karma threshold') text = config.get('testing_approval_msg_based_on_karma') update.comment(db, text, author=u'bodhi') db.commit() @@ -99,7 +99,7 @@ def main(argv=sys.argv): # this function only needs to consider the time requirements because these updates have # not reached the karma threshold. if update.meets_testing_requirements: - print('%s now meets testing requirements' % update.title) + print(f'{update.alias} now meets testing requirements') text = str( config.get('testing_approval_msg') % update.mandatory_days_in_testing) update.comment(db, text, author=u'bodhi') diff --git a/bodhi/server/scripts/untag_branched.py b/bodhi/server/scripts/untag_branched.py index d6ae4e57c2..801d701df5 100644 --- a/bodhi/server/scripts/untag_branched.py +++ b/bodhi/server/scripts/untag_branched.py @@ -1,4 +1,4 @@ -# Copyright © 2015-2018 Red Hat, Inc. and others. +# Copyright © 2015-2019 Red Hat, Inc. and others. # # This file is part of Bodhi. # @@ -78,7 +78,6 @@ def main(argv=sys.argv): log.info(release.name) for update in db.query(Update).filter_by( release=release, status=UpdateStatus.stable).all(): - assert update.date_stable, update.title if now - update.date_stable > one_day: for build in update.builds: tags = build.get_tags() diff --git a/bodhi/server/services/builds.py b/bodhi/server/services/builds.py index 7ebab6bfce..e8ebef83fc 100644 --- a/bodhi/server/services/builds.py +++ b/bodhi/server/services/builds.py @@ -1,4 +1,4 @@ -# Copyright © 2014-2017 Red Hat, Inc. and others. +# Copyright © 2014-2019 Red Hat, Inc. and others. # # This file is part of Bodhi. # @@ -98,9 +98,7 @@ def query_builds(request): updates = data.get('updates') if updates is not None: query = query.join(Build.update) - args = \ - [Update.title == update.title for update in updates] +\ - [Update.alias == update.alias for update in updates] + args = [Update.alias == update.alias for update in updates] query = query.filter(or_(*args)) packages = data.get('packages') diff --git a/bodhi/server/services/releases.py b/bodhi/server/services/releases.py index b17baf6de8..e88ada7d8d 100644 --- a/bodhi/server/services/releases.py +++ b/bodhi/server/services/releases.py @@ -1,4 +1,4 @@ -# Copyright © 2014-2018 Red Hat Inc., and others. +# Copyright © 2014-2019 Red Hat Inc., and others. # # This file is part of Bodhi. # @@ -249,9 +249,7 @@ def query_releases_json(request): updates = data.get('updates') if updates is not None: query = query.join(Release.builds).join(Build.update) - args = \ - [Update.title == update.title for update in updates] +\ - [Update.alias == update.alias for update in updates] + args = [Update.alias == update.alias for update in updates] query = query.filter(or_(*args)) packages = data.get('packages') diff --git a/bodhi/server/services/updates.py b/bodhi/server/services/updates.py index 09232a5b12..8cf84e01a3 100644 --- a/bodhi/server/services/updates.py +++ b/bodhi/server/services/updates.py @@ -108,7 +108,7 @@ error_handler=bodhi.server.services.errors.html_handler) def get_update(request): """ - Return a single update from an id, title, or alias. + Return a single update from an id or alias. Args: request (pyramid.request): The current request. @@ -133,7 +133,7 @@ def get_update(request): )) def get_update_for_editing(request): """ - Return a single update from an id, title, or alias for the edit form. + Return a single update from an id or alias for the edit form. Args: request (pyramid.request): The current request. @@ -274,14 +274,13 @@ def query_updates(request): like = data.get('like') if like is not None: - query = query.filter(or_(*[ - Update.title.like('%%%s%%' % like) - ])) + query = query.join(Update.builds) + query = query.filter(Build.nvr.like('%%%s%%' % like)) search = data.get('search') if search is not None: - query = query.filter(or_(Update.title.ilike('%%%s%%' % search), - Update.alias.ilike('%%%s%%' % search))) + query = query.join(Update.builds) + query = query.filter(Build.nvr.ilike('%%%s%%' % search)) locked = data.get('locked') if locked is not None: @@ -422,7 +421,7 @@ def new_update(request): Save an update. This entails either creating a new update, or editing an existing one. To - edit an existing update, the update's original title must be specified in + edit an existing update, the update's alias must be specified in the ``edited`` parameter. Args: @@ -505,7 +504,7 @@ def new_update(request): log.info('Creating new update: %r' % _data['builds']) result, _caveats = Update.new(request, _data) - log.debug('%s update created', result.title) + log.debug('%s update created', result.alias) updates.append(result) caveats.extend(_caveats) diff --git a/bodhi/server/services/user.py b/bodhi/server/services/user.py index cfad59ede1..5cd6583010 100644 --- a/bodhi/server/services/user.py +++ b/bodhi/server/services/user.py @@ -1,4 +1,4 @@ -# Copyright 2014-2018 Red Hat, Inc. and others +# Copyright 2014-2019 Red Hat, Inc. and others # # This file is part of Bodhi. # @@ -153,9 +153,7 @@ def query_users(request): updates = data.get('updates') if updates is not None: query = query.join(User.updates) - args = \ - [Update.title == update.title for update in updates] +\ - [Update.alias == update.alias for update in updates] + args = [Update.alias == update.alias for update in updates] query = query.filter(or_(*args)) packages = data.get('packages') diff --git a/bodhi/server/static/js/search.js b/bodhi/server/static/js/search.js index b7ce363e75..cb6f98d5ff 100644 --- a/bodhi/server/static/js/search.js +++ b/bodhi/server/static/js/search.js @@ -51,8 +51,6 @@ $(document).ready(function() { function resultUrl(data){ if (data.alias != undefined) { return '/updates/' + data.alias; - } else if (data.title != undefined ) { - return '/updates/' + data.title; } else if (data.hasOwnProperty('stack')) { return'/updates/?packages=' + encodeURIComponent(data.name); } else if (data.name != undefined) { diff --git a/bodhi/server/static/js/update_form.js b/bodhi/server/static/js/update_form.js index c44d6438bd..5925d69aa0 100644 --- a/bodhi/server/static/js/update_form.js +++ b/bodhi/server/static/js/update_form.js @@ -15,7 +15,7 @@ $(document).ready(function() { if (data.updates === undefined) { // Single-release update // Now redirect to the update display - document.location.href = base + "updates/" + data.title; + document.location.href = base + "updates/" + data.alias; } else { // Multi-release update // Redirect to updates created by *me* diff --git a/bodhi/server/templates/new_update.html b/bodhi/server/templates/new_update.html index 5776cd1a03..5d92fc2506 100644 --- a/bodhi/server/templates/new_update.html +++ b/bodhi/server/templates/new_update.html @@ -26,16 +26,7 @@

New Update % else:

Edit - % if update.alias: ${update.alias} - % else: - ${update.get_title(', ', 2, ", …") | n} -
- - ${ "(+ " + str(len(update.builds) - 2) + " more)" if len(update.builds) > 2 else "" } - -
- % endif

% endif @@ -297,7 +288,7 @@

Edit % if update: - + % endif