Skip to content
Permalink
Browse files

PEP-8 bodhi.server.models.models.

This commit also adds a new test for enforcing that the models.py
file stays compliant with PEP-8 with two exceptions: 100 character
line length and ignoring one particular flake8 error that is very
common for sqlalchemy projects. The plan is to add new modules to
this test as the code base is converted to be PEP-8 compliant over
time, until the entire code base is enforced.
  • Loading branch information...
bowlofeggs committed Oct 5, 2016
1 parent 79b024f commit bbafc9e6c1869725204aa2d0678974f0eedc982b
Showing with 122 additions and 80 deletions.
  1. +75 −80 bodhi/server/models/models.py
  2. +45 −0 bodhi/tests/test_style.py
  3. +1 −0 devel/ansible/roles/dev/tasks/main.yml
  4. +1 −0 setup.py
@@ -189,13 +189,12 @@ def update_relationship(self, name, model, data, db):
return new, same, removed



Base = declarative_base(cls=BodhiBase)
metadata = Base.metadata


##
## Enumerated type declarations
# Enumerated type declarations
##

class UpdateStatus(DeclEnum):
@@ -244,35 +243,28 @@ class ReleaseState(DeclEnum):


##
## Association tables
##

#update_release_table = Table('update_release_table', metadata,
# Column('release_id', Integer, ForeignKey('releases.id')),
# Column('update_id', Integer, ForeignKey('updates.id')))

#update_build_table = Table('update_build_table', metadata,
# Column('update_id', Integer, ForeignKey('updates.id')),
# Column('build_id', Integer, ForeignKey('builds.id')))


# Association tables
##

update_bug_table = Table('update_bug_table', metadata,
Column('update_id', Integer, ForeignKey('updates.id')),
Column('bug_id', Integer, ForeignKey('bugs.id')))
update_bug_table = Table(
'update_bug_table', metadata,
Column('update_id', Integer, ForeignKey('updates.id')),
Column('bug_id', Integer, ForeignKey('bugs.id')))

update_cve_table = Table('update_cve_table', metadata,
Column('update_id', Integer, ForeignKey('updates.id')),
Column('cve_id', Integer, ForeignKey('cves.id')))
update_cve_table = Table(
'update_cve_table', metadata,
Column('update_id', Integer, ForeignKey('updates.id')),
Column('cve_id', Integer, ForeignKey('cves.id')))

bug_cve_table = Table('bug_cve_table', metadata,
Column('bug_id', Integer, ForeignKey('bugs.id')),
Column('cve_id', Integer, ForeignKey('cves.id')))
bug_cve_table = Table(
'bug_cve_table', metadata,
Column('bug_id', Integer, ForeignKey('bugs.id')),
Column('cve_id', Integer, ForeignKey('cves.id')))

user_package_table = Table('user_package_table', metadata,
Column('user_id', Integer, ForeignKey('users.id')),
Column('package_id', Integer, ForeignKey('packages.id')))
user_package_table = Table(
'user_package_table', metadata,
Column('user_id', Integer, ForeignKey('users.id')),
Column('package_id', Integer, ForeignKey('packages.id')))


class Release(Base):
@@ -437,7 +429,7 @@ def list_categorymembers(wiki, cat_page, limit=500):
cmtitle=cat_page, cmlimit=limit)
response = wiki.call(query)
members = [entry['title'] for entry in
response.get('query',{}).get('categorymembers',{})
response.get('query', {}).get('categorymembers', {})
if 'title' in entry]

# Determine whether we need to recurse
@@ -447,8 +439,8 @@ def list_categorymembers(wiki, cat_page, limit=500):
break
# Recurse?
if members[idx].startswith('Category:') and limit > 0:
members.extend(list_categorymembers(wiki, members[idx], limit-1))
members.remove(members[idx]) # remove Category from list
members.extend(list_categorymembers(wiki, members[idx], limit - 1))
members.remove(members[idx]) # remove Category from list
else:
idx += 1

@@ -524,7 +516,7 @@ def get_latest(self):
evr = self.evr
for tag in [self.update.release.stable_tag, self.update.release.dist_tag]:
builds = koji_session.getLatestBuilds(
tag, package=self.package.name)
tag, package=self.package.name)

# Find the first build that is older than us
for build in builds:
@@ -596,10 +588,12 @@ def unpush(self, koji):
log.info('Removing %s tag from %s' % (tag, self.nvr))
koji.untagBuild(tag, self.nvr)
elif tag == release.testing_tag:
log.info('Moving %s from %s to %s' % (self.nvr, tag,
release.candidate_tag))
log.info(
'Moving %s from %s to %s' % (
self.nvr, tag, release.candidate_tag))
koji.moveBuild(tag, release.candidate_tag, self.nvr)


class Update(Base):
__tablename__ = 'updates'
__exclude_columns__ = ('id', 'user_id', 'release_id', 'cves')
@@ -663,11 +657,6 @@ class Update(Base):
bugs = relationship('Bug', secondary=update_bug_table, backref='updates')
cves = relationship('CVE', secondary=update_cve_table, backref='updates')

# We may or may not need this, since we can determine the releases from the
# builds
#releases = relationship('Release', secondary=update_release_table,
# backref='updates', lazy=False)

user_id = Column(Integer, ForeignKey('users.id'))

@classmethod
@@ -828,7 +817,7 @@ def edit(cls, request, data):
data['request'] = UpdateRequest.testing

# Remove all koji tags and change the status back to pending
if not up.status is UpdateStatus.pending:
if up.status is not UpdateStatus.pending:
up.unpush(db)
caveats.append({
'name': 'status',
@@ -953,8 +942,8 @@ def get_tags(self):

def get_title(self, delim=' ', limit=None, after_limit=''):
all_nvrs = map(lambda x: x.nvr, self.builds)
nvrs = all_nvrs[:limit]
builds = delim.join(sorted(nvrs)) + (after_limit if limit and len(all_nvrs) > limit else "")
nvrs = all_nvrs[:limit]
builds = delim.join(sorted(nvrs)) + (after_limit if limit and len(all_nvrs) > limit else "")
return builds

def get_bugstring(self, show_titles=False):
@@ -963,8 +952,8 @@ def get_bugstring(self, show_titles=False):
if show_titles:
i = 0
for bug in self.bugs:
bugstr = u'%s%s - %s\n' % (i and ' ' * 11 + ': ' or '',
bug.bug_id, bug.title)
bugstr = u'%s%s - %s\n' % (
i and ' ' * 11 + ': ' or '', bug.bug_id, bug.title)
val += u'\n'.join(wrap(
bugstr, width=67,
subsequent_indent=' ' * 11 + ': ')) + '\n'
@@ -1085,19 +1074,20 @@ def set_request(self, db, action, username):
if action is UpdateRequest.stable and self.critpath:
if config.get('critpath.num_admin_approvals') is not None:
if not self.critpath_approved:
notes.append('This critical path update has not '
'yet been approved for pushing to the stable '
'repository. It must first reach a karma '
'of %s, consisting of %s positive karma from '
'proventesters, along with %d additional '
'karma from the community. Or, it must '
'spend %s days in testing without any '
'negative feedback'
% (config.get('critpath.min_karma'),
config.get('critpath.num_admin_approvals'),
int(config.get('critpath.min_karma')) -
int(config.get('critpath.num_admin_approvals')),
config.get('critpath.stable_after_days_without_negative_karma')))
stern_note = (
'This critical path update has not yet been approved for pushing to the '
'stable repository. It must first reach a karma of %s, consisting of %s '
'positive karma from proventesters, along with %d additional karma from '
'the community. Or, it must spend %s days in testing without any negative '
'feedback')
stern_note = stern_note % (
config.get('critpath.min_karma'),
config.get('critpath.num_admin_approvals'),
(int(config.get('critpath.min_karma')) -
int(config.get('critpath.num_admin_approvals'))),
config.get('critpath.stable_after_days_without_negative_karma'))
notes.append(stern_note)

if self.status is UpdateStatus.testing:
self.request = None
raise BodhiException('. '.join(notes))
@@ -1110,7 +1100,7 @@ def set_request(self, db, action, username):
if action is UpdateRequest.stable and not self.critpath:
# 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:
self.stable_karma) or self.critpath_approved:
log.debug('%s meets stable karma requirements' % self.title)
else:
# If we haven't met the stable karma requirements, check if it
@@ -1141,15 +1131,16 @@ def set_request(self, db, action, username):
# it to the pending state, and make sure it's tagged as a candidate
if self.status in (UpdateStatus.obsolete, UpdateStatus.unpushed):
self.status = UpdateStatus.pending
if not self.release.candidate_tag in self.get_tags():
if self.release.candidate_tag not in self.get_tags():
self.add_tag(self.release.candidate_tag)

self.request = action

notes = notes and '. '.join(notes) + '.' or ''
flash_notes = flash_notes and '. %s' % flash_notes
flash_log("%s has been submitted for %s. %s%s" % (self.title,
action.description, notes, flash_notes))
flash_log(
"%s has been submitted for %s. %s%s" % (
self.title, 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
@@ -1251,7 +1242,7 @@ def send_update_notice(self):
mailinglist = config.get('%s_announce_list' % release_name)
elif self.status is UpdateStatus.testing:
mailinglist = config.get('%s_test_announce_list' % release_name)

# switch email template to legacy if update aims to EPEL <= 7
if release_name == 'fedora_epel' and self.release.version_int <= 7:
templatetype = '%s_errata_legacy_template' % release_name
@@ -1261,7 +1252,8 @@ def send_update_notice(self):
if mailinglist:
for subject, body in mail.get_template(self, templatetype):
mail.send_mail(sender, mailinglist, subject, body)
notifications.publish(topic='errata.publish',
notifications.publish(
topic='errata.publish',
msg=dict(subject=subject, body=body, update=self))
else:
log.error("Cannot find mailing list address for update notice")
@@ -1299,7 +1291,7 @@ def __str__(self):
self.type.description, self.karma)
if self.critpath:
val += u"\n Critpath: %s" % self.critpath
if self.request != None:
if self.request is not None:
val += u"\n Request: %s" % self.request.description
if len(self.bugs):
bugs = self.get_bugstring(show_titles=True)
@@ -1411,8 +1403,9 @@ def obsolete_if_unstable(self, db):
If an update with pending status(autopush enabled) reaches unstable karma
threshold, make sure it gets obsoleted.
"""
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:
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)
self.obsolete(db)
flash_log("%s has been obsoleted." % self.title)
@@ -1447,8 +1440,8 @@ def comment(self, session, text, karma=0, author=None, anonymous=False,
# Take all comments since the previous karma reset
reset_index = 0
for i, comment in enumerate(self.comments):
if (comment.user.name == u'bodhi' and ('New build' in
comment.text or 'Removed build' in comment.text)):
if (comment.user.name == u'bodhi' and
('New build' in comment.text or 'Removed build' in comment.text)):
reset_index = i

mycomments = [c.karma for c in self.comments if c.user.name == author][reset_index:]
@@ -1694,8 +1687,9 @@ def check_karma_thresholds(self, db, agent):
msg=dict(update=self, status='stable'))
else:
# 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)
log.info((
"%s update has reached the stable karma threshold and can be pushed to "
"stable now if the maintainer wishes") % self.title)
elif self.unstable_karma not in (0, None) and self.karma <= self.unstable_karma:
log.info("Automatically unpushing %s" % self.title)
self.obsolete(db)
@@ -1758,15 +1752,14 @@ def critpath_approved(self):
status = config.get('%s.status' % release_name, None)
if status:
num_admin_approvals = config.get('%s.%s.critpath.num_admin_approvals' % (
release_name, status), None)
release_name, status), None)
min_karma = config.get('%s.%s.critpath.min_karma' % (
release_name, status), None)
release_name, status), None)
if num_admin_approvals is not None and min_karma:
return self.num_admin_approvals >= int(num_admin_approvals) and \
self.karma >= int(min_karma)
return self.num_admin_approvals >= int(config.get(
'critpath.num_admin_approvals', 2)) and \
self.karma >= int(config.get('critpath.min_karma', 2))
self.karma >= int(min_karma)
return self.num_admin_approvals >= int(config.get('critpath.num_admin_approvals', 2)) and \
self.karma >= int(config.get('critpath.min_karma', 2))

@property
def meets_testing_requirements(self):
@@ -1822,7 +1815,9 @@ def met_testing_requirements(self):
def days_to_stable(self):
""" Return the number of days until an update can be pushed to stable """
if self.meets_testing_requirements:
return self.release.mandatory_days_in_testing - (datetime.utcnow() - self.date_testing).days
return (
self.release.mandatory_days_in_testing -
(datetime.utcnow() - self.date_testing).days)
else:
return 0

@@ -1909,7 +1904,6 @@ def __json__(self, request=None, anonymize=False):
return result



# Used for many-to-many relationships between karma and a bug
class BugKarma(Base):
__tablename__ = 'comment_bug_assoc'
@@ -1991,7 +1985,7 @@ class CVE(Base):
@property
def url(self):
return "http://www.cve.mitre.org/cgi-bin/cvename.cgi?name=%s" % \
self.cve_id
self.cve_id


class Bug(Base):
@@ -2175,9 +2169,10 @@ def new(cls, request, **data):
'%s is already in a override' % build.nvr)
return

old_build = db.query(Build).filter(and_(
Build.package_id==build.package_id,
Build.release_id==build.release_id)).first()
old_build = db.query(Build).filter(
and_(
Build.package_id == build.package_id,
Build.release_id == build.release_id)).first()

if old_build is not None and old_build.override is not None:
# There already is a buildroot override for an older build of this
@@ -0,0 +1,45 @@
# -*- coding: utf-8 -*-

# 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.
"""This test suite enforces code style automatically."""

import os
import subprocess
import unittest


REPO_PATH = os.path.abspath(
os.path.dirname(os.path.join(os.path.dirname(__file__), '..', '..', '..')))


class TestStyle(unittest.TestCase):
"""This test class contains tests pertaining to code style."""
def test_code_with_flake8(self):
"""Enforce PEP-8 compliance on the codebase.
This test runs flake8 on a subset of the code. The bodhi code is currently undergoing a
change to bring all of the codebase into PEP-8 compliance, but the changes will be made
slowly. This test enforces only modules that have been corrected to comply with flake8. The
goal is that this test would one day check the entire codebase.
"""
enforced_paths = ['bodhi/server/models/models.py']

enforced_paths = [os.path.join(REPO_PATH, p) for p in enforced_paths]

# We ignore E712, which disallows non-identity comparisons with True and False
flake8_command = ['/usr/bin/flake8', '--max-line-length', '100', '--ignore=E712']
flake8_command.extend(enforced_paths)

self.assertEqual(subprocess.call(flake8_command), 0)
@@ -28,6 +28,7 @@
- python-zmq
- python2-createrepo_c
- python2-fedmsg-atomic-composer
- python2-flake8
- redhat-rpm-config
- zlib-devel

0 comments on commit bbafc9e

Please sign in to comment.
You can’t perform that action at this time.