diff --git a/bodhi/client/__init__.py b/bodhi/client/__init__.py index 577c62b859..da1532b074 100644 --- a/bodhi/client/__init__.py +++ b/bodhi/client/__init__.py @@ -420,7 +420,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. @@ -430,11 +430,10 @@ def _validate_edit_update(ctx, param, value): Raises: click.BadParameter: If the value is invalid. """ - if re.search(bindings.UPDATE_ID_RE, value)\ - or re.search(bindings.UPDATE_TITLE_RE, value): + if re.search(bindings.UPDATE_ID_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() @@ -451,11 +450,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. @@ -472,19 +471,9 @@ def edit(user, password, url, debug, openid_api, **kwargs): kwargs['notes'] = _get_notes(**kwargs) try: - 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'] + query_param = {'updateid': kwargs['update']} + resp = client.query(**query_param) 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() @@ -492,6 +481,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: @@ -507,7 +499,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') @@ -593,7 +584,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. @@ -641,7 +632,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 """ @@ -719,7 +710,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 @@ -783,13 +774,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. @@ -1016,7 +1007,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: @@ -1034,7 +1025,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 cece8ffb9a..be4ea2d4a2 100644 --- a/bodhi/client/bindings.py +++ b/bodhi/client/bindings.py @@ -224,7 +224,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: @@ -248,7 +248,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``). @@ -275,7 +275,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: @@ -301,7 +301,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 @@ -328,7 +328,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. @@ -374,9 +373,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'] @@ -414,7 +410,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. @@ -427,7 +423,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 @@ -796,16 +792,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']), @@ -900,12 +895,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/composer.py b/bodhi/server/consumers/composer.py index b079391d35..890dac83bd 100644 --- a/bodhi/server/consumers/composer.py +++ b/bodhi/server/consumers/composer.py @@ -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)) @@ -384,7 +376,7 @@ def work(self): notifications.publish( topic="compose.composing", 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, @@ -470,7 +462,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_compose(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 @@ -487,7 +479,7 @@ def eject_from_compose(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 @@ -722,7 +714,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 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/metadata.py b/bodhi/server/metadata.py index ef57c5b879..02faac318e 100644 --- a/bodhi/server/metadata.py +++ b/bodhi/server/metadata.py @@ -146,8 +146,6 @@ def __init__(self, release, request, db, composedir, 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: 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 96879c0b72..1d38df812a 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" @@ -1626,10 +1622,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=False) @@ -1667,7 +1661,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) @@ -1695,6 +1689,25 @@ class Update(Base): # Greenwave test_gating_status = Column(TestGatingStatus.db_type(), default=None, nullable=True) + def __init__(self, *args, **kwargs): + """ + Initialize the Update. + + We use this as a way to inject an alias into the Update, since it is a required field and + we don't want callers to have to generate the alias themselves. + """ + super(Update, self).__init__(*args, **kwargs) + + # Let's give this Update an alias so the DB doesn't become displeased with us. + if not self.release: + raise ValueError('You must specify a Release when creating an Update.') + prefix = self.release.id_prefix + 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.get_title(), alias)) + self.alias = alias + @property def side_tag_locked(self): """ @@ -1954,7 +1967,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) @@ -1988,18 +2000,8 @@ def new(cls, request, data): # Create the update log.debug("Creating new Update(**data) object.") release = data.pop('release', None) - up = Update(**data) - # Autoflush will cause a problem for Update.validate_release(). - # https://github.com/fedora-infra/bodhi/issues/2117 - with util.no_autoflush(db): - up.release = release - - # Assign the alias before setting the request. - # Setting the request publishes a fedmsg message, and it is nice to - # already have the alias there for URL construction and backend update - # handling. - log.debug("Assigning alias for new update..") - up.assign_alias() + up = Update(**data, release=release) + log.debug("Setting request for new update.") up.set_request(db, req, request.user.name) @@ -2032,7 +2034,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 = [] @@ -2098,8 +2100,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 @@ -2279,6 +2279,15 @@ def get_tags(self): """ return list(set(sum([b.get_tags() for b in self.builds], []))) + @property + def title(self) -> str: + """ + Return the Update's title. + + This is just an alias for get_title with default parameters. + """ + 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. @@ -2403,19 +2412,6 @@ def build_label(build): else: return " and ".join([build_label(build) for build in self.builds]) - def assign_alias(self): - """Return a randomly-suffixed update ID. - - This function used to construct update IDs in a monotonic sequence, but - we ran into race conditions so we do it randomly now. - """ - prefix = self.release.id_prefix - 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)) - self.alias = alias - def set_request(self, db, action, username): """ Set the update's request to the given action. @@ -2438,10 +2434,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 @@ -2455,11 +2451,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 @@ -2470,7 +2466,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 @@ -2480,14 +2476,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 @@ -2525,7 +2521,7 @@ def set_request(self, db, action, username): if action == UpdateRequest.stable and not self.critpath: # Check if we've met the karma requirements if 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 @@ -2564,7 +2560,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 @@ -2627,9 +2623,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() @@ -2651,9 +2647,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 @@ -2673,12 +2669,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: @@ -2710,7 +2706,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: @@ -2743,10 +2739,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): @@ -2769,10 +2762,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 @@ -2866,9 +2857,9 @@ def obsolete_if_unstable(self, db): if self.autokarma and self.status is UpdateStatus.pending \ and self.request is UpdateRequest.testing\ 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, @@ -2935,9 +2926,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: @@ -3003,11 +2994,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: @@ -3031,7 +3022,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( @@ -3060,7 +3051,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: @@ -3084,7 +3075,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 @@ -3222,7 +3213,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): @@ -3236,7 +3227,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( @@ -3246,12 +3237,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 +3489,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): @@ -3671,7 +3662,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 @@ -3682,8 +3673,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) @@ -3926,8 +3916,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 f2b70080ee..4f0f5be131 100644 --- a/bodhi/server/push.py +++ b/bodhi/server/push.py @@ -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 0a8f3104de..78b6e48375 100644 --- a/bodhi/server/scripts/approve_testing.py +++ b/bodhi/server/scripts/approve_testing.py @@ -87,7 +87,7 @@ def main(argv=sys.argv): # Approval message when testing based on karma threshold if 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 22c62a91d6..e60ab9eb51 100644 --- a/bodhi/server/scripts/untag_branched.py +++ b/bodhi/server/scripts/untag_branched.py @@ -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..8afaadc8fb 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,14 @@ 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(or_( + Build.nvr.ilike('%%%s%%' % search), Update.alias.ilike('%%%s%%' % search))) locked = data.get('locked') if locked is not None: @@ -422,7 +422,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 +505,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