From d57696c8013d78f017972ae72efeeb441c954cb2 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Sat, 28 Aug 2010 11:37:39 -0500 Subject: PyLint suggested cleanups We had a bunch of extra imports, non-conventional variable names, spacing issues, etc. that were relatively low-hanging fruit to clean up. Fix them and make the code a bit cleaner in the process. Signed-off-by: Dan McGee --- devel/management/commands/reporead.py | 52 +++++++++++++++++------------------ devel/views.py | 34 ++++++++++++++--------- feeds.py | 3 +- main/models.py | 5 ++-- mirrors/views.py | 1 - packages/views.py | 29 +++++++++---------- public/utils.py | 14 ++++++---- public/views.py | 2 +- urls.py | 2 -- 9 files changed, 75 insertions(+), 67 deletions(-) diff --git a/devel/management/commands/reporead.py b/devel/management/commands/reporead.py index 598c0194..ee0c50fb 100644 --- a/devel/management/commands/reporead.py +++ b/devel/management/commands/reporead.py @@ -22,11 +22,9 @@ REPOVARS = ['arch', 'backup', 'base', 'builddate', 'conflicts', 'csize', from django.core.management.base import BaseCommand, CommandError -from django.conf import settings from django.contrib.auth.models import User -from django.db import models, transaction +from django.db import transaction from django.db.models import Q -from django.core import management import os import re @@ -39,7 +37,7 @@ from optparse import make_option from cStringIO import StringIO from logging import ERROR, WARNING, INFO, DEBUG -from main.models import Arch, Package, Repo, UserProfile +from main.models import Arch, Package, Repo class SomethingFishyException(Exception): '''Raised when the database looks like its going to wipe out a bunch of @@ -63,15 +61,15 @@ class Command(BaseCommand): help = "Runs a package repository import for the given arch and file." args = " " - def handle(self, arch=None, file=None, **options): + def handle(self, arch=None, filename=None, **options): if not arch: raise CommandError('Architecture is required.') if not validate_arch(arch): raise CommandError('Specified architecture %s is not currently known.' % arch) - if not file: + if not filename: raise CommandError('Package database file is required.') - file = os.path.normpath(file) - if not os.path.exists(file) or not os.path.isfile(file): + filename = os.path.normpath(filename) + if not os.path.exists(filename) or not os.path.isfile(filename): raise CommandError('Specified package database file does not exist.') v = int(options.get('verbosity', 0)) @@ -82,17 +80,17 @@ class Command(BaseCommand): elif v == 2: logger.level = DEBUG - import signal,traceback + import signal, traceback signal.signal(signal.SIGQUIT, lambda sig, stack: traceback.print_stack(stack)) - return read_repo(arch, file, options) + return read_repo(arch, filename, options) class Pkg(object): """An interim 'container' object for holding Arch package data.""" - def __init__(self, val): + def __init__(self, val, repo): selfdict = {} squash = ['arch', 'builddate', 'csize', 'desc', 'filename', 'installdate', 'isize', 'license', 'md5sum', @@ -106,7 +104,7 @@ class Pkg(object): for x in val.keys(): if x in squash: if val[x] == None or len(val[x]) == 0: - logger.warning("Package %s has no %s" % (selfdict['name'],x)) + logger.warning("Package %s has no %s" % (selfdict['name'], x)) selfdict[x] = None else: selfdict[x] = ', '.join(val[x]) @@ -127,8 +125,9 @@ class Pkg(object): else: selfdict[x] = val[x] self.__dict__ = selfdict + self.repo = repo - def __getattr__(self,name): + def __getattr__(self, name): if name == 'force': return False else: @@ -196,11 +195,13 @@ def populate_pkg(dbpkg, repopkg, force=False, timestamp=None): dbpkg.installed_size = int(repopkg.isize) try: dbpkg.build_date = datetime.utcfromtimestamp(int(repopkg.builddate)) - except: + except ValueError: try: - dbpkg.build_date = datetime.strptime(repopkg.builddate, '%a %b %d %H:%M:%S %Y') - except: - pass + dbpkg.build_date = datetime.strptime(repopkg.builddate, + '%a %b %d %H:%M:%S %Y') + except ValueError: + logger.warning('Package %s had unparsable build date %s' % \ + (repopkg.name, repopkg.builddate)) dbpkg.packager_str = repopkg.packager # attempt to find the corresponding django user for this string dbpkg.packager = find_user(repopkg.packager) @@ -217,12 +218,12 @@ def populate_pkg(dbpkg, repopkg, force=False, timestamp=None): for y in repopkg.depends: # make sure we aren't adding self depends.. # yes *sigh* i have seen them in pkgbuilds - dpname,dpvcmp = re.match(r"([a-z0-9._+-]+)(.*)", y).groups() + dpname, dpvcmp = re.match(r"([a-z0-9._+-]+)(.*)", y).groups() if dpname == repopkg.name: logger.warning('Package %s has a depend on itself' % repopkg.name) continue dbpkg.packagedepend_set.create(depname=dpname, depvcmp=dpvcmp) - logger.debug('Added %s as dep for pkg %s' % (dpname,repopkg.name)) + logger.debug('Added %s as dep for pkg %s' % (dpname, repopkg.name)) dbpkg.packagegroup_set.all().delete() if 'groups' in repopkg.__dict__: @@ -323,7 +324,7 @@ def db_update(archname, reponame, pkgs, options): # for a non-force, we don't want to do anything at all. if filesonly: pass - elif ''.join((p.ver,p.rel)) == ''.join((dbp.pkgver,dbp.pkgrel)): + elif '-'.join((p.ver, p.rel)) == '-'.join((dbp.pkgver, dbp.pkgrel)): if not force: continue else: @@ -349,16 +350,16 @@ def parse_inf(iofile): store = {} lines = iofile.readlines() blockname = None - max = len(lines) + max_len = len(lines) i = 0 - while i < max: + while i < max_len: line = lines[i].strip() if len(line) > 0 and line[0] == '%' and line[1:-1].lower() in REPOVARS: blockname = line[1:-1].lower() - logger.debug("Parsing package block %s",blockname) + logger.debug("Parsing package block %s", blockname) store[blockname] = [] i += 1 - while i < max and len(lines[i].strip()) > 0: + while i < max_len and len(lines[i].strip()) > 0: store[blockname].append(lines[i].strip()) i += 1 # here is where i would convert arrays to strings @@ -402,8 +403,7 @@ def parse_repo(repopath): if tpkg != None: tpkg.reset() data = parse_inf(tpkg) - p = Pkg(data) - p.repo = reponame + p = Pkg(data, reponame) logger.debug("Done parsing package %s", p.name) pkgs.append(p) if tarinfo == None: diff --git a/devel/views.py b/devel/views.py index 192a4572..baa5fd6c 100644 --- a/devel/views.py +++ b/devel/views.py @@ -6,19 +6,16 @@ from django.contrib.sites.models import Site from django.shortcuts import render_to_response from django.template import RequestContext from django.core.mail import send_mail -from django.db.models import Q from django.views.decorators.cache import never_cache from main.models import Package, Todolist, TodolistPkg from main.models import Arch, Repo -from main.models import UserProfile, News +from main.models import UserProfile from main.models import Mirror from packages.models import PackageRelation import random from string import ascii_letters, digits -pwletters = ascii_letters + digits - @login_required @never_cache @@ -30,13 +27,16 @@ def index(request): todopkgs = TodolistPkg.objects.select_related( 'pkg', 'pkg__arch', 'pkg__repo').filter(complete=False) - todopkgs = todopkgs.filter(pkg__pkgbase__in=inner_q).order_by('list__name', 'pkg__pkgname') + todopkgs = todopkgs.filter(pkg__pkgbase__in=inner_q).order_by( + 'list__name', 'pkg__pkgname') + maintainers = User.objects.filter(is_active=True).order_by( + 'first_name', 'last_name') page_dict = { 'todos': Todolist.objects.incomplete().order_by('-date_added'), 'repos': Repo.objects.all(), 'arches': Arch.objects.all(), - 'maintainers': User.objects.filter(is_active=True).order_by('first_name', 'last_name'), + 'maintainers': maintainers, 'flagged' : flagged, 'todopkgs' : todopkgs, } @@ -48,9 +48,9 @@ def index(request): def change_notify(request): maint = User.objects.get(username=request.user.username) notify = request.POST.get('notify', 'no') - pf = maint.get_profile() - pf.notify = (notify == 'yes') - pf.save() + prof = maint.get_profile() + prof.notify = (notify == 'yes') + prof.save() return HttpResponseRedirect('/devel/') class ProfileForm(forms.Form): @@ -98,6 +98,7 @@ class NewUserForm(forms.ModelForm): def save(self): profile = forms.ModelForm.save(self, False) + pwletters = ascii_letters + digits pw = ''.join([random.choice(pwletters) for i in xrange(8)]) user = User.objects.create_user(username=self.cleaned_data['username'], email=self.cleaned_data['email'], password=pw) @@ -125,14 +126,21 @@ def new_user_form(request): form = NewUserForm(request.POST) if form.is_valid(): form.save() - return HttpResponseRedirect('/admin/auth/user/%d/' %form.instance.user.id) + return HttpResponseRedirect('/admin/auth/user/%d/' % \ + form.instance.user.id) else: form = NewUserForm() - return render_to_response('general_form.html', RequestContext( - request, {'description': '''A new user will be created with the + + context = { + 'description': '''A new user will be created with the following properties in their profile. A random password will be generated and the user will be e-mailed with their account details n plaintext.''', - 'form': form, 'title': 'Create User', 'submit_text': 'Create User'})) + 'form': form, + 'title': 'Create User', + 'submit_text': 'Create User' + } + return render_to_response('general_form.html', + RequestContext(request, context)) # vim: set ts=4 sw=4 et: diff --git a/feeds.py b/feeds.py index 05c00ca0..37aa6aaa 100644 --- a/feeds.py +++ b/feeds.py @@ -1,6 +1,5 @@ import datetime -from django.contrib.syndication.feeds import FeedDoesNotExist from django.contrib.syndication.views import Feed from django.db.models import Q from main.models import Arch, Repo, Package, News @@ -53,7 +52,7 @@ class PackageFeed(Feed): return item.last_update def item_categories(self, item): - return (item.repo.name,item.arch.name) + return (item.repo.name, item.arch.name) class NewsFeed(Feed): diff --git a/main/models.py b/main/models.py index c6b4e139..2079deeb 100644 --- a/main/models.py +++ b/main/models.py @@ -1,5 +1,4 @@ from django.db import models -from django.db.models import Q from django.contrib.auth.models import User from django.contrib.sites.models import Site @@ -138,7 +137,7 @@ class News(models.Model): class Arch(models.Model): id = models.AutoField(primary_key=True) - name = models.CharField(max_length=255,unique=True) + name = models.CharField(max_length=255, unique=True) def __unicode__(self): return self.name class Meta: @@ -148,7 +147,7 @@ class Arch(models.Model): class Repo(models.Model): id = models.AutoField(primary_key=True) - name = models.CharField(max_length=255,unique=True) + name = models.CharField(max_length=255, unique=True) testing = models.BooleanField(default=False, help_text="Is this repo meant for package testing?") bugs_project = models.SmallIntegerField(default=1, diff --git a/mirrors/views.py b/mirrors/views.py index 7ef8becd..378a0b42 100644 --- a/mirrors/views.py +++ b/mirrors/views.py @@ -1,5 +1,4 @@ from django import forms -from django.db.models import Q from django.shortcuts import render_to_response from django.template import RequestContext from django.views.decorators.csrf import csrf_exempt diff --git a/packages/views.py b/packages/views.py index 4fd34edf..2fe515c5 100644 --- a/packages/views.py +++ b/packages/views.py @@ -20,20 +20,20 @@ from main.models import Package, PackageFile from main.models import Arch, Repo, Signoff from main.models import MirrorUrl from main.utils import make_choice -from .models import PackageGroup, PackageRelation +from .models import PackageRelation from .utils import get_group_info, get_differences_info def opensearch(request): if request.is_secure(): - d = "https://%s" % request.META['HTTP_HOST'] + domain = "https://%s" % request.META['HTTP_HOST'] else: - d = "http://%s" % request.META['HTTP_HOST'] + domain = "http://%s" % request.META['HTTP_HOST'] response = HttpResponse(mimetype='application/opensearchdescription+xml') - t = loader.get_template('packages/opensearch.xml') - c = Context({ - 'domain': d, + template = loader.get_template('packages/opensearch.xml') + ctx = Context({ + 'domain': domain, }) - response.write(t.render(c)) + response.write(template.render(ctx)) return response @permission_required('main.change_package') @@ -54,11 +54,11 @@ def update(request): for pkg in pkgs: maints = pkg.maintainers if mode == 'adopt' and request.user not in maints: - pr = PackageRelation(pkgbase=pkg.pkgbase, + prel = PackageRelation(pkgbase=pkg.pkgbase, user=request.user, type=PackageRelation.MAINTAINER) count += 1 - pr.save() + prel.save() elif mode == 'disown' and request.user in maints: rels = PackageRelation.objects.filter(pkgbase=pkg.pkgbase, user=request.user) @@ -86,9 +86,9 @@ def details(request, name='', repo='', arch=''): arch.lower(), repo.title(), name)) def groups(request): - groups = get_group_info() + grps = get_group_info() return render_to_response('packages/groups.html', - RequestContext(request, {'groups': groups})) + RequestContext(request, {'groups': grps})) def group_details(request, arch, name): arch = get_object_or_404(Arch, name=arch) @@ -157,7 +157,7 @@ class PackageSearchForm(forms.Form): def search(request, page=None): current_query = '?' - limit=50 + limit = 50 packages = Package.objects.select_related('arch', 'repo') if request.GET: @@ -224,11 +224,12 @@ def search(request, page=None): def files(request, name='', repo='', arch=''): pkg = get_object_or_404(Package, pkgname=name, repo__name__iexact=repo, arch__name=arch) - files = PackageFile.objects.filter(pkg=pkg).order_by('path') + fileslist = PackageFile.objects.filter(pkg=pkg).order_by('path') template = 'packages/files.html' if request.is_ajax(): template = 'packages/files-list.html' - return render_to_response(template, RequestContext(request, {'pkg':pkg,'files':files})) + return render_to_response(template, RequestContext(request, + {'pkg':pkg, 'files':fileslist})) @permission_required('main.change_package') def unflag(request, name='', repo='', arch=''): diff --git a/public/utils.py b/public/utils.py index c92cb264..2801c939 100644 --- a/public/utils.py +++ b/public/utils.py @@ -1,4 +1,6 @@ -from main.models import Arch, Repo, Package +from operator import attrgetter + +from main.models import Arch, Package from main.utils import cache_function @cache_function(300) @@ -7,18 +9,20 @@ def get_recent_updates(): # want to try and eliminate cross-architecture wasted space. Pull enough # packages that we can later do some screening and trim out the fat. pkgs = [] - for a in Arch.objects.all(): + for arch in Arch.objects.all(): # grab a few extra so we can hopefully catch everything we need - pkgs += list(Package.objects.select_related('arch', 'repo').filter(arch=a).order_by('-last_update')[:50]) + pkgs += list(Package.objects.select_related( + 'arch', 'repo').filter(arch=arch).order_by('-last_update')[:50]) pkgs.sort(key=lambda q: q.last_update) updates = [] ctr = 0 while ctr < 15 and len(pkgs) > 0: # not particularly happy with this logic, but it works. p = pkgs.pop() - samepkgs = filter(lambda q: p.is_same_version(q) and p.repo == q.repo, pkgs) + is_same = lambda q: p.is_same_version(q) and p.repo == q.repo + samepkgs = filter(is_same, pkgs) samepkgs.append(p) - samepkgs.sort(key=lambda q: q.arch.name) + samepkgs.sort(key=attrgetter('arch.name')) updates.append(samepkgs) for q in samepkgs: if p != q: pkgs.remove(q) diff --git a/public/views.py b/public/views.py index f2c649f9..be0ff80d 100644 --- a/public/views.py +++ b/public/views.py @@ -1,5 +1,5 @@ from main.models import Arch, Donor, MirrorUrl, News -from main.models import Package, Repo +from main.models import Repo from . import utils from django.contrib.auth.models import User diff --git a/urls.py b/urls.py index 0030343c..3f394426 100644 --- a/urls.py +++ b/urls.py @@ -2,9 +2,7 @@ from django.conf.urls.defaults import * from django.conf import settings from django.contrib import admin -from django.views.generic.create_update import delete_object from django.views.generic.simple import direct_to_template -from django.contrib.auth.decorators import permission_required from main.models import Todolist from feeds import PackageFeed, NewsFeed -- cgit v1.2.3-2-g168b