Merge pull request #26 from MasterChief-John-117/pylint

Add pylint and config for pylint and travis

.pylintrc

@@ -0,0 +1,101 @@
+[MASTER]
+ignore = ,input
+persistent = yes
+
+[MESSAGES CONTROL]
+disable =
+    missing-docstring,
+	fixme,
+	duplicate-code,
+	no-member,
+	parse-error,
+	bad-continuation,
+	too-few-public-methods,
+	global-statement,
+	cyclic-import,
+	locally-disabled,
+	file-ignored,
+	no-else-return,
+	unnecessary-lambda,
+	wrong-import-position,
+	logging-format-interpolation,
+	bare-except,
+	too-many-public-methods
+
+[REPORTS]
+output-format = text
+files-output = no
+reports = no
+
+[FORMAT]
+max-line-length = 120
+max-statement-lines = 75
+single-line-if-stmt = no
+no-space-check = trailing-comma,dict-separator
+max-module-lines = 1000
+indent-string = '    '
+string-quote=single-avoid-escape
+triple-quote=single
+docstring-quote=double
+
+[MISCELLANEOUS]
+notes = FIXME,XXX,TODO
+
+[SIMILARITIES]
+min-similarity-lines = 4
+ignore-comments = yes
+ignore-docstrings = yes
+ignore-imports = no
+
+[BASIC]
+# Regular expression which should only match correct module names
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
+
+# Regular expression which should only match correct module level names
+const-rgx=(([A-Za-z_][A-Za-z1-9_]*)|(__.*__))$
+
+# Regular expression which should only match correct class names
+class-rgx=[A-Z_][a-zA-Z0-9_]+$
+
+# Regular expression which should only match correct function names
+function-rgx=[a-z_][a-z0-9_]{2,35}$
+
+# Regular expression which should only match correct method names
+method-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct instance attribute names
+attr-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct argument names
+argument-rgx=[a-z_][a-z0-9_]{0,30}$
+
+# Regular expression which should only match correct variable names
+variable-rgx=[a-z_][a-z0-9_]{0,30}$
+
+# Regular expression which should only match correct list comprehension /
+# generator expression variable names
+inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$
+
+# Good variable names which should always be accepted, separated by a comma
+good-names=logger,id,ID
+
+# Bad variable names which should always be refused, separated by a comma
+bad-names=foo,bar,baz,toto,tutu,tata
+
+# List of builtins function names that should not be used, separated by a comma
+bad-functions=apply,input
+
+[DESIGN]
+max-args = 10
+ignored-argument-names = _.*
+max-locals = 20
+max-returns = 6
+max-branches = 15
+max-statements = 55
+max-parents = 7
+max-attributes = 10
+min-public-methods = 2
+max-public-methods = 20
+
+[EXCEPTIONS]
+overgeneral-exceptions = Exception

.travis.yml

@@ -0,0 +1,8 @@
+language: python
+python:
+  - "3.6"
+
+install:
+  - "pip install -r requirements.txt"
+script:
+  - "pylint proxstar"

proxstar/__init__.py

@@ -1,11 +1,11 @@
 import os
 import json
 import time
-import psutil
 import atexit
 import logging
-import psycopg2
 import subprocess
+import psutil
+import psycopg2
 import rq_dashboard
 from rq import Queue
 from redis import Redis
@@ -13,13 +13,15 @@ from rq_scheduler import Scheduler
 from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker
 from flask import Flask, render_template, request, redirect, session, abort
-from proxstar.db import *
+from proxstar.db import (Base, datetime, get_pool_cache, renew_vm_expire, set_user_usage_limits, get_template,
-from proxstar.vnc import *
+get_templates, get_allowed_users, add_ignored_pool, delete_ignored_pool, add_allowed_user, delete_allowed_user,
+get_template_disk, set_template_info, url_for)
+from proxstar.vnc import (send_stop_ssh_tunnel, stop_ssh_tunnel, add_vnc_target, start_ssh_tunnel, get_vnc_targets,
+delete_vnc_target, stop_websockify)
 from proxstar.auth import get_auth
 from proxstar.util import gen_password
-from proxstar.starrs import *
+from proxstar.starrs import check_hostname, renew_ip
-from proxstar.ldapdb import *
+from proxstar.proxmox import connect_proxmox, get_isos, get_pools, get_ignored_pools
-from proxstar.proxmox import *
 
 logging.basicConfig(
     format='%(asctime)s %(levelname)s %(message)s', level=logging.INFO)
@@ -37,8 +39,8 @@ app.config.from_pyfile(config)
 app.config["GIT_REVISION"] = subprocess.check_output(
     ['git', 'rev-parse', '--short', 'HEAD']).decode('utf-8').rstrip()
 
-with open('proxmox_ssh_key', 'w') as key:
+with open('proxmox_ssh_key', 'w') as ssh_key_file:
-    key.write(app.config['PROXMOX_SSH_KEY'])
+    ssh_key_file.write(app.config['PROXMOX_SSH_KEY'])
 
 ssh_tunnels = []
 
@@ -60,7 +62,8 @@ starrs = psycopg2.connect(
 
 from proxstar.vm import VM
 from proxstar.user import User
-from proxstar.tasks import generate_pool_cache_task, process_expiring_vms_task, cleanup_vnc_task, delete_vm_task, create_vm_task, setup_template_task
+from proxstar.tasks import (generate_pool_cache_task, process_expiring_vms_task, cleanup_vnc_task,
+delete_vm_task, create_vm_task, setup_template_task)
 
 if 'generate_pool_cache' not in scheduler:
     logging.info('adding generate pool cache task to scheduler')
@@ -87,7 +90,7 @@ if 'cleanup_vnc' not in scheduler:
 def add_rq_dashboard_auth(blueprint):
     @blueprint.before_request
     @auth.oidc_auth
-    def rq_dashboard_auth(*args, **kwargs):
+    def rq_dashboard_auth(*args, **kwargs): #pylint: disable=unused-argument,unused-variable
         if 'rtp' not in session['userinfo']['groups']:
             abort(403)
 
@@ -100,13 +103,13 @@ app.register_blueprint(rq_dashboard_blueprint, url_prefix="/rq")
 @app.errorhandler(404)
 def not_found(e):
     user = User(session['userinfo']['preferred_username'])
-    return render_template('404.html', user=user), 404
+    return render_template('404.html', user=user, e=e), 404
 
 
 @app.errorhandler(403)
 def forbidden(e):
     user = User(session['userinfo']['preferred_username'])
-    return render_template('403.html', user=user), 403
+    return render_template('403.html', user=user, e=e), 403
 
 
 @app.route("/")
@@ -115,7 +118,7 @@ def forbidden(e):
 def list_vms(user_view=None):
     user = User(session['userinfo']['preferred_username'])
     rtp_view = False
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user_view and not user.rtp:
         abort(403)
     elif user_view and user.rtp:
@@ -155,8 +158,8 @@ def list_vms(user_view=None):
 @auth.oidc_auth
 def isos():
     proxmox = connect_proxmox()
-    isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE'])
+    stored_isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE'])
-    return json.dumps({"isos": isos})
+    return json.dumps({"isos": stored_isos})
 
 
 @app.route("/hostname/<string:name>")
@@ -175,7 +178,7 @@ def hostname(name):
 @auth.oidc_auth
 def vm_details(vmid):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         usage_check = user.check_usage(vm.cpu, vm.mem, 0)
@@ -187,19 +190,19 @@ def vm_details(vmid):
             limits=user.limits,
             usage_check=usage_check)
     else:
-        abort(403)
+        return abort(403)
 
 
 @app.route("/vm/<string:vmid>/power/<string:action>", methods=['POST'])
 @auth.oidc_auth
 def vm_power(vmid, action):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         if action == 'start':
-            config = vm.config
+            vmconfig = vm.config
-            usage_check = user.check_usage(config['cores'], config['memory'],
+            usage_check = user.check_usage(vmconfig['cores'], vmconfig['memory'],
                                            0)
             if usage_check:
                 return usage_check
@@ -235,7 +238,7 @@ def vm_console_stop(vmid):
 @auth.oidc_auth
 def vm_console(vmid):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         stop_ssh_tunnel(vm.id, ssh_tunnels)
@@ -255,7 +258,7 @@ def vm_console(vmid):
 @auth.oidc_auth
 def vm_cpu(vmid, cores):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         cur_cores = vm.cpu
@@ -276,7 +279,7 @@ def vm_cpu(vmid, cores):
 @auth.oidc_auth
 def vm_mem(vmid, mem):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         cur_mem = vm.mem // 1024
@@ -297,10 +300,9 @@ def vm_mem(vmid, mem):
 @auth.oidc_auth
 def vm_disk(vmid, disk, size):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
-        cur_cores = vm.cpu
         usage_check = user.check_usage(0, 0, size)
         if usage_check:
             return usage_check
@@ -314,7 +316,7 @@ def vm_disk(vmid, disk, size):
 @auth.oidc_auth
 def vm_renew(vmid):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         renew_vm_expire(db, vmid, app.config['VM_EXPIRE_MONTHS'])
@@ -330,7 +332,7 @@ def vm_renew(vmid):
 @auth.oidc_auth
 def iso_eject(vmid):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         vm = VM(vmid)
         vm.eject_iso()
@@ -343,7 +345,7 @@ def iso_eject(vmid):
 @auth.oidc_auth
 def iso_mount(vmid, iso):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         iso = "{}:iso/{}".format(app.config['PROXMOX_ISO_STORAGE'], iso)
         vm = VM(vmid)
@@ -357,7 +359,7 @@ def iso_mount(vmid, iso):
 @auth.oidc_auth
 def delete(vmid):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         send_stop_ssh_tunnel(vmid)
         # Submit the delete VM task to RQ
@@ -369,9 +371,9 @@ def delete(vmid):
 
 @app.route("/vm/<string:vmid>/boot_order", methods=['POST'])
 @auth.oidc_auth
-def boot_order(vmid):
+def get_boot_order(vmid):
     user = User(session['userinfo']['preferred_username'])
-    proxmox = connect_proxmox()
+    connect_proxmox()
     if user.rtp or int(vmid) in user.allowed_vms:
         boot_order = []
         for key in sorted(request.form):
@@ -390,7 +392,7 @@ def create():
     proxmox = connect_proxmox()
     if user.active or user.rtp:
         if request.method == 'GET':
-            isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE'])
+            stored_isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE'])
             pools = get_pools(proxmox, db)
             templates = get_templates(db)
             return render_template(
@@ -399,7 +401,7 @@ def create():
                 usage=user.usage,
                 limits=user.limits,
                 percents=user.usage_percent,
-                isos=isos,
+                isos=stored_isos,
                 pools=pools,
                 templates=templates)
         elif request.method == 'POST':
@@ -449,6 +451,7 @@ def create():
                             timeout=600)
                         return '', 200
             return '', 200
+        return None
     else:
         return '', 403
 
@@ -470,7 +473,7 @@ def set_limits(user):
 @auth.oidc_auth
 def delete_user(user):
     if 'rtp' in session['userinfo']['groups']:
-        proxmox = connect_proxmox()
+        connect_proxmox()
         User(user).delete()
         return '', 200
     else:
@@ -483,16 +486,16 @@ def settings():
     user = User(session['userinfo']['preferred_username'])
     if user.rtp:
         templates = get_templates(db)
-        ignored_pools = get_ignored_pools(db)
+        db_ignored_pools = get_ignored_pools(db)
-        allowed_users = get_allowed_users(db)
+        db_allowed_users = get_allowed_users(db)
         return render_template(
             'settings.html',
             user=user,
             templates=templates,
-            ignored_pools=ignored_pools,
+            ignored_pools=db_ignored_pools,
-            allowed_users=allowed_users)
+            allowed_users=db_allowed_users)
     else:
-        abort(403)
+        return abort(403)
 
 
 @app.route("/pool/<string:pool>/ignore", methods=['POST', 'DELETE'])

proxstar/auth.py

@@ -8,5 +8,4 @@ def get_auth(app):
         app,
         issuer=app.config['OIDC_ISSUER'],
         client_registration_info=app.config['OIDC_CLIENT_CONFIG'])
-    auth
     return auth

proxstar/db.py

@@ -3,8 +3,8 @@ import datetime
 from dateutil.relativedelta import relativedelta
 from sqlalchemy import exists
 
-from proxstar.ldapdb import *
+from proxstar.ldapdb import is_rtp
-from proxstar.models import (Allowed_Users, Base, Ignored_Pools, Pool_Cache,
+from proxstar.models import (Allowed_Users, Ignored_Pools, Pool_Cache,
                              Template, Usage_Limit, VM_Expiration)
 
 

proxstar/ldapdb.py

@@ -34,7 +34,7 @@ def is_current_student(user):
 def is_user(user):
     ldap = connect_ldap()
     try:
-        rtp_group = ldap.get_member(user, uid=True)
+        ldap.get_member(user, uid=True)
         return True
     except:
         return False

proxstar/proxmox.py

@@ -2,7 +2,7 @@ from flask import current_app as app
 from proxmoxer import ProxmoxAPI
 
 from proxstar import logging
-from proxstar.db import get_ignored_pools, get_user_usage_limits
+from proxstar.db import get_ignored_pools
 from proxstar.ldapdb import is_user
 
 
@@ -14,7 +14,7 @@ def connect_proxmox():
                 user=app.config['PROXMOX_USER'],
                 password=app.config['PROXMOX_PASS'],
                 verify_ssl=False)
-            version = proxmox.version.get()
+            proxmox.version.get()
             return proxmox
         except:
             if app.config['PROXMOX_HOSTS'].index(host) == (
@@ -33,7 +33,7 @@ def connect_proxmox_ssh():
                 private_key_file='proxmox_ssh_key',
                 password=app.config['PROXMOX_SSH_KEY_PASS'],
                 backend='ssh_paramiko')
-            version = proxmox.version.get()
+            proxmox.version.get()
             return proxmox
         except:
             if app.config['PROXMOX_HOSTS'].index(host) == (
@@ -58,6 +58,7 @@ def get_vm_node(proxmox, vmid):
     for vm in proxmox.cluster.resources.get(type='vm'):
         if vm['vmid'] == int(vmid):
             return vm['node']
+    return None
 
 
 def get_isos(proxmox, storage):

proxstar/starrs.py

@@ -72,7 +72,7 @@ def check_hostname(starrs, hostname):
         if c.fetchall():
             available = False
         c.execute("COMMIT")
-    except (psycopg2.InternalError):
+    except psycopg2.InternalError:
         valid = False
         available = False
     finally:

proxstar/tasks.py

@@ -2,7 +2,6 @@ import logging
 import os
 import time
 
-import paramiko
 import psycopg2
 import requests
 from flask import Flask
@@ -10,12 +9,11 @@ from rq import get_current_job
 from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker
 
-from proxstar.db import *
+from proxstar.db import Base, get_vm_expire, delete_vm_expire, datetime, store_pool_cache, get_template
-from proxstar.mail import *
+from proxstar.mail import send_vm_expire_email, send_rtp_vm_delete_email
 from proxstar.proxmox import connect_proxmox, get_pools
-from proxstar.starrs import *
+from proxstar.starrs import get_next_ip, register_starrs, delete_starrs
 from proxstar.user import User, get_vms_for_rtp
-from proxstar.util import *
 from proxstar.vm import VM, clone_vm, create_vm
 from proxstar.vnc import send_stop_ssh_tunnel
 
@@ -36,8 +34,8 @@ app.config.from_pyfile(config)
 def connect_db():
     engine = create_engine(app.config['SQLALCHEMY_DATABASE_URI'])
     Base.metadata.bind = engine
-    DBSession = sessionmaker(bind=engine)
+    dbsession = sessionmaker(bind=engine)
-    db = DBSession()
+    db = dbsession()
     return db
 
 
@@ -114,7 +112,7 @@ def process_expiring_vms_task():
     with app.app_context():
         proxmox = connect_proxmox()
         db = connect_db()
-        starrs = connect_starrs()
+        connect_starrs()
         pools = get_pools(proxmox, db)
         expired_vms = []
         for pool in pools:
@@ -125,7 +123,6 @@ def process_expiring_vms_task():
                 vm = VM(vm['vmid'])
                 days = (vm.expire - datetime.date.today()).days
                 if days in [10, 7, 3, 1, 0, -1, -2, -3, -4, -5, -6]:
-                    name = vm.name
                     expiring_vms.append([vm.id, vm.name, days])
                     if days <= 0:
                         expired_vms.append([vm.id, vm.name, days])
@@ -158,7 +155,7 @@ def setup_template_task(template_id, name, user, ssh_key, cores, memory):
         db = connect_db()
         logging.info("[{}] Retrieving template info for template {}.".format(
             name, template_id))
-        template = get_template(db, template_id)
+        get_template(db, template_id)
         logging.info("[{}] Cloning template {}.".format(name, template_id))
         set_job_status(job, 'cloning template')
         vmid = clone_vm(proxmox, template_id, name, user)

proxstar/user.py

@@ -2,13 +2,13 @@ from proxmoxer.core import ResourceException
 from rq.registry import StartedJobRegistry
 
 from proxstar import db, q, redis_conn
-from proxstar.db import *
+from proxstar.db import get_allowed_users, is_active, is_current_student, is_rtp, is_user, get_user_usage_limits
-from proxstar.proxmox import *
+from proxstar.proxmox import connect_proxmox, get_pools
-from proxstar.util import *
+from proxstar.util import lazy_property
 from proxstar.vm import VM
 
 
-class User(object):
+class User():
     def __init__(self, username):
         self.name = username
         self.active = is_active(self.name) or is_current_student(
@@ -96,11 +96,12 @@ class User(object):
     def check_usage(self, cpu, mem, disk):
         if int(self.usage['cpu']) + int(cpu) > int(self.limits['cpu']):
             return 'exceeds_cpu_limit'
-        elif int(self.usage['mem']) + (int(mem) / 1024) > int(
+        elif int(self.usage['mem']) + (int(mem) / 1024) > int(self.limits['mem']):
-                self.limits['mem']):
             return 'exceeds_memory_limit'
         elif int(self.usage['disk']) + int(disk) > int(self.limits['disk']):
             return 'exceeds_disk_limit'
+        else:
+            return None
 
     def delete(self):
         proxmox = connect_proxmox()
@@ -114,9 +115,9 @@ class User(object):
                     self.name)).delete()
 
 
-def get_vms_for_rtp(proxmox, db):
+def get_vms_for_rtp(proxmox, database):
     pools = []
-    for pool in get_pools(proxmox, db):
+    for pool in get_pools(proxmox, database):
         user = User(pool)
         pool_dict = dict()
         pool_dict['user'] = user.name

proxstar/util.py

@@ -1,5 +1,4 @@
 import random
-import string
 
 
 def gen_password(

proxstar/vm.py

@@ -1,5 +1,4 @@
 import json
-import time
 import urllib
 
 from flask import current_app as app
@@ -12,7 +11,7 @@ from proxstar.starrs import get_ip_for_mac
 from proxstar.util import lazy_property
 
 
-class VM(object):
+class VM():
     def __init__(self, vmid):
         self.id = vmid
 
@@ -52,6 +51,7 @@ class VM(object):
         for vm in proxmox.cluster.resources.get(type='vm'):
             if vm['vmid'] == int(self.id):
                 return vm['node']
+        return None
 
     @retry(wait=wait_fixed(2), stop=stop_after_attempt(5))
     def delete(self):
@@ -127,8 +127,8 @@ class VM(object):
         }
         raw_boot_order = self.config.get('boot', 'cdn')
         boot_order = []
-        for i in range(0, len(raw_boot_order)):
+        for order in raw_boot_order:
-            boot_order.append(boot_order_lookup[raw_boot_order[i]])
+            boot_order.append(boot_order_lookup[order])
         return boot_order
 
     @lazy_property
@@ -145,14 +145,14 @@ class VM(object):
             'Network': 'n'
         }
         raw_boot_order = ''
-        for i in range(0, len(boot_order)):
+        for order in boot_order:
-            raw_boot_order += boot_order_lookup[boot_order[i]]
+            raw_boot_order += boot_order_lookup[order]
         proxmox.nodes(self.node).qemu(self.id).config.put(boot=raw_boot_order)
 
     @lazy_property
     def interfaces(self):
         interfaces = []
-        for key, val in self.config.items():
+        for key, _ in self.config.items():
             if 'net' in key:
                 mac = self.config[key].split(',')
                 valid_int_types = ['virtio', 'e1000', 'rtl8139', 'vmxnet3']

proxstar/vnc.py

@@ -7,7 +7,7 @@ from flask import current_app as app
 from sshtunnel import SSHTunnelForwarder
 
 from proxstar import logging
-from proxstar.util import *
+from proxstar.util import gen_password
 
 
 def stop_websockify():

requirements.txt

@@ -17,3 +17,5 @@ sqlalchemy==1.2.12
 sshtunnel==0.1.4
 tenacity==5.0.2
 websockify==0.8.0
+pylint==2.3.1
+pylint-quotes==0.2.1