Merge remote-tracking branch 'origin/master'

Conflicts:
	slapos/runner/gittools.py

CHANGES.txt

+0.37.4 (2013-10-15)
+-------------------
+
+ * Improve QEMU QMP wrapper by adding drive-backup method and other helpers. [0afb7d6, 95d0c8b]
+
+0.37.3 (2013-10-10)
+-------------------
+
+* pubsub: don't swallow output of subprocess to allow debug. [c503484]
+
+0.37.2 (2013-10-10)
+-------------------
+
+ * Add QEMU QMP wrapper. [9e819a8]
+ * KVM resiliency test: update docstring about how to setup disk image. [dbe347f]
+ * KVM resiliency test: change key for each clone. [7ef1db3]
+
+0.37.1 (2013-10-03)
+-------------------
+
+  * pubsub notifier: handle timeout and other connection errors. [ac4c75c]
+  * equeue: cast str(timestamp) to please gdbm. [8b067d6]
+
+0.37 (2013-09-30)
+=================
+
+ * equeue: log output of subprocess. [1694937]
+ * slaprunner: don't send 200 when login is bad. [4a8e10bf]
+ * Improve reliability of resiliency tests.
+
 0.36 (2013-09-05)
 =================
 

setup.py

@@ -2,7 +2,7 @@ from setuptools import setup, find_packages
 import glob
 import os
 
-version = '0.36.1-dev'
+version = '0.37.4'
 name = 'slapos.toolbox'
 long_description = open("README.txt").read() + "\n" + \
     open("CHANGES.txt").read() + "\n"
@@ -70,6 +70,7 @@ setup(name=name,
           'onetimeupload = slapos.onetimeupload:main',
           'pubsubnotifier = slapos.pubsub.notifier:main',
           'pubsubserver = slapos.pubsub:main',
+          'qemu-qmp-client = slapos.qemuqmpclient:main',
           'shacache = slapos.shacache:main',
           'slapbuilder = slapos.builder:main',
           'slapcontainer = slapos.container:main',

slapos/equeue/__init__.py

@@ -72,6 +72,26 @@ class EqueueServer(SocketServer.ThreadingUnixStreamServer):
   def setDB(self, database):
     self.db = gdbm.open(database, 'cs', 0700)
 
+  def _runCommandIfNeeded(self, command, timestamp):
+    with self.lock:
+      if command in self.db and timestamp <= int(self.db[command]):
+        self.logger.info("%s already run.", command)
+        return
+
+      self.logger.info("Running %s, %s with output:", command, timestamp)
+      try:
+        self.logger.info(
+            subprocess.check_output([command], stderr=subprocess.STDOUT)
+        )
+        self.logger.info("%s finished successfully.", command)
+      except subprocess.CalledProcessError as e:
+        self.logger.warning("%s exited with status %s. output is: \n %s" % (
+            command,
+            e.returncode,
+            e.output,
+        ))
+      self.db[command] = str(timestamp)
+
   def process_request_thread(self, request, client_address):
     # Handle request
     self.logger.debug("Connection with file descriptor %d", request.fileno())
@@ -102,23 +122,7 @@ class EqueueServer(SocketServer.ThreadingUnixStreamServer):
     except:
       self.logger.warning("Couldn't respond to %r", request.fileno())
     self.close_request(request)
-    # Run command if needed
-    with self.lock:
-      if command not in self.db or timestamp > int(self.db[command]):
-        self.logger.info("Running %s, %s", command, timestamp)
-        # XXX stdout and stderr redirected to null as they are not read
-        with open(os.devnull, 'r+') as f:
-          status = subprocess.call([command], close_fds=True,
-                                   stdin=f, stdout=f, stderr=f)
-        if status:
-          self.logger.warning("%s finished with non zero status.",
-                              command)
-        else:
-          self.logger.info("%s finished successfully.", command)
-        self.db[command] = timestamp
-      else:
-        self.logger.info("%s already runned.", command)
-
+    self._runCommandIfNeeded(command, timestamp)
 # Well the following function is made for schrodinger's files,
 # It will work if the file exists or not
 def remove_existing_file(path):

slapos/pubsub/notifier.py

@@ -4,11 +4,11 @@
 import argparse
 import csv
 import httplib
-import os
 import socket
 import subprocess
 import sys
 import time
+import traceback
 import urllib2
 import urlparse
 import uuid
@@ -31,28 +31,23 @@ def main():
 
   args = parser.parse_args()
 
-  with open(os.devnull) as devnull:
-    command = subprocess.Popen(args.executable[0],
-                               stdin=subprocess.PIPE,
-                               stdout=devnull,
-                               stderr=subprocess.PIPE,
-                               close_fds=True)
-    command.stdin.flush()
-    command.stdin.close()
-
-    command_failed = (command.wait() != 0)
-    command_stderr = command.stderr.read()
-
-    if command_failed:
-      content = ("<p>Failed with returncode <em>%d</em>.</p>"
-                 "<p>Standard error output is :</p><pre>%s</pre>") % (
-        command.poll(),
-        command_stderr.replace('&', '&amp;')\
-                      .replace('<', '&lt;')\
-                      .replace('>', '&gt;'),
+  try:
+    content = subprocess.check_output(
+        args.executable[0],
+        stderr=subprocess.STDOUT
     )
-    else:
-      content = "<p>Everything went well.</p>"
+    exit_code = 0
+  except subprocess.CalledProcessError as e:
+    content = e.output
+    exit_code = e.returncode
+
+  print content
+
+  content += ("\n<p>Failed with returncode <em>%d</em>.</p>"
+              "<p>Output is: </p><pre>%s</pre>" % (
+      exit_code,
+      content.replace('&', '&amp;').replace('<', '&lt;').replace('>', '&gt;')
+  ))
 
   with open(args.logfile[0], 'a') as file_:
     cvsfile = csv.writer(file_)
@@ -63,9 +58,8 @@ def main():
       'slapos:%s' % uuid.uuid4(),
     ])
 
-  if command_failed:
-    sys.stderr.write('%s\n' % command_stderr)
-    sys.exit(1)
+  if  exit_code != 0:
+    sys.exit(exit_code)
 
   print 'Fetching %s feed...' % args.feed_url[0]
 
@@ -80,15 +74,19 @@ def main():
       notification_port = socket.getservbyname(notification_url.scheme)
 
     headers = {'Content-Type': feed.info().getheader('Content-Type')}
+    try:
       notification = httplib.HTTPConnection(notification_url.hostname,
                                             notification_port)
       notification.request('POST', notification_url.path, body, headers)
       response = notification.getresponse()
-
       if not (200 <= response.status < 300):
         sys.stderr.write("The remote server at %s didn't send a successful reponse.\n" % notif_url)
         sys.stderr.write("Its response was %r\n" % response.reason)
         some_notification_failed = True
+    except socket.error as exc:
+      sys.stderr.write("Connection with remote server at %s failed:\n" % notif_url)
+      sys.stderr.write(traceback.format_exc(exc))
+      some_notification_failed = True
 
   if some_notification_failed:
     sys.exit(1)

slapos/qemuqmpclient/__init__.py

+##############################################################################
+#
+# Copyright (c) 2013 Vifib SARL and Contributors. All Rights Reserved.
+#
+# WARNING: This program as such is intended to be used by professional
+# programmers who take the whole responsibility of assessing all potential
+# consequences resulting from its eventual inadequacies and bugs
+# End users who are looking for a ready-to-use solution with commercial
+# guarantees and support are strongly adviced to contract a Free Software
+# Service Company
+#
+# 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 3
+# 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., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+#
+##############################################################################
+
+import argparse
+import json
+import os
+import pprint
+import socket
+import time
+
+def parseArgument():
+  """
+  Very basic argument parser. Might blow up for anything else than
+  "./executable mysocket.sock stop/resume".
+  """
+  parser = argparse.ArgumentParser()
+  parser.add_argument('--suspend', action='store_const', dest='action', const='suspend')
+  parser.add_argument('--resume', action='store_const', dest='action', const='resume')
+  parser.add_argument('--create-snapshot', action='store_const', dest='action', const='createSnapshot')
+  parser.add_argument('--create-internal-snapshot', action='store_const', dest='action', const='createInternalSnapshot')
+  parser.add_argument('--delete-internal-snapshot', action='store_const', dest='action', const='deleteInternalSnapshot')
+  parser.add_argument('--drive-backup', action='store_const', dest='action', const='driveBackup') 
+  parser.add_argument('--query-commands', action='store_const', dest='action', const='queryCommands')
+
+  parser.add_argument('--socket', dest='unix_socket_location', required=True)
+  parser.add_argument('remainding_argument_list', nargs=argparse.REMAINDER)
+  args = parser.parse_args()
+  return args.unix_socket_location, args.action, args.remainding_argument_list
+
+
+class QemuQMPWrapper(object):
+  """
+  Small wrapper around Qemu's QMP to control a qemu VM.
+  See http://git.qemu.org/?p=qemu.git;a=blob;f=qmp-commands.hx for
+  QMP API definition.
+  """
+  def __init__(self, unix_socket_location):
+    self.socket = self.connectToQemu(unix_socket_location)
+    self.capabilities()
+
+  @staticmethod
+  def connectToQemu(unix_socket_location):
+    """
+    Create a socket, connect to qemu, be sure it answers, return socket.
+    """
+    if not os.path.exists(unix_socket_location):
+      raise Exception('unix socket %s does not exist.' % unix_socket_location)
+
+    print 'Connecting to qemu...'
+    so = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    connected = False
+    while not connected:
+      try:
+        so.connect(unix_socket_location)
+      except socket.error:
+        time.sleep(1)
+        print 'Could not connect, retrying...'
+      else:
+        connected = True
+    so.recv(1024)
+
+    return so
+
+
+  def _send(self, message):
+    self.socket.send(json.dumps(message))
+    data = self.socket.recv(65535)
+    try:
+      return json.loads(data)
+    except ValueError:
+      print 'Wrong data: %s' % data
+
+  def _getVMStatus(self):
+    response = self._send({'execute': 'query-status'})
+    if response:
+      return self._send({'execute': 'query-status'})['return']['status']
+    else:
+      raise IOError('Empty answer')
+
+  def _waitForVMStatus(self, wanted_status):
+    while True:
+      try:
+        actual_status = self._getVMStatus()
+        if actual_status == wanted_status:
+          return
+        else:
+          print 'VM in %s status, wanting it to be %s, retrying...' % (
+              actual_status, wanted_status)
+          time.sleep(1)
+      except IOError:
+          print 'VM not ready, retrying...'
+
+
+  def capabilities(self):
+    print 'Asking for capabilities...'
+    self._send({'execute': 'qmp_capabilities'})
+
+  def suspend(self):
+    print 'Suspending VM...'
+    self._send({'execute': 'stop'})
+    self._waitForVMStatus('paused')
+
+  def resume(self):
+    print 'Resuming VM...'
+    self._send({'execute': 'cont'})
+    self._waitForVMStatus('running')
+
+  def _queryBlockJobs(self, device):
+    return self._send({'execute': 'query-block-jobs'})
+
+  def _getRunningJobList(self, device):
+    result = self._queryBlockJobs(device)
+    if result.get('return'):
+      return result['return']
+    else:
+      return
+
+  def driveBackup(self, backup_target, source_device='virtio0', sync_type='full'):
+    print 'Asking Qemu to perform backup to %s' % backup_target
+    # XXX: check for error
+    self._send({
+        'execute': 'drive-backup',
+        'arguments': {
+            'device': source_device,
+            'sync': sync_type,
+            'target': backup_target,
+         }
+    })
+    while self._getRunningJobList(backup_target):
+      print 'Job is not finished yet.'
+      time.sleep(20)
+
+  def createSnapshot(self, snapshot_file, device='virtio0'):
+    print self._send({
+        'execute': 'blockdev-snapshot-sync',
+        'arguments': {
+            'device': device,
+            'snapshot-file': snapshot_file,
+         }
+    })
+
+  def createInternalSnapshot(self, name=None, device='virtio0'):
+    if name is None:
+      name = int(time.time())
+    self._send({
+        'execute': 'blockdev-snapshot-internal-sync',
+        'arguments': {
+            'device': device,
+            'name': name,
+         }
+    })
+
+  def deleteInternalSnapshot(self, name, device='virtio0'):
+    self._send({
+        'execute': 'blockdev-snapshot-delete-internal-sync',
+        'arguments': {
+            'device': device,
+            'name': name,
+         }
+    })
+
+  def queryCommands(self):
+    pprint.pprint(self._send({'execute': 'query-commands'})['return'])
+
+def main():
+  unix_socket_location, action, remainding_argument_list = parseArgument()
+  qemu_wrapper = QemuQMPWrapper(unix_socket_location)
+
+  if remainding_argument_list:
+    getattr(qemu_wrapper, action)(*remainding_argument_list)
+  else:
+    getattr(qemu_wrapper, action)()
+
+if __name__ == '__main__':
+  main()
+

slapos/resiliencytest/README.txt

@@ -15,3 +15,32 @@ This module contains:
  * A Resiliency Test Suite framework (in suites/), used to easily write new
    test suites
  * A list of test suites
+
+
+
+TODO :
+ * Check that each partition is in different slapos node.
+ * Test for bang calls
+ * Be able to configure from ERP5 Master (i.e from instance parameters): count of PBS/clones, then test several possibilities (so called "count" in test suite)
+ * Use Nexedi ERP5, when in production.
+ * Put the KVM disk image in a safe place.
+
+------------
+
+For reference: How-to deploy the whole test system
+1/ Deploy a SlapOS Master
+2/ Deploy an ERP5, install erp5_test_result BT with scalability feature (current in scalability-master2 branch of erp5.git) (currently, had to change a few lines in the scalability extension of the portal_class, should be commited)
+3/ Configure 3 nodes in the new SlapOS Master, deploy in each a testnode with scalability feature (erp5testnode-scalability branch of slapos.git) with parameters like:
+<?xml version="1.0" encoding="utf-8"?>
+<instance>
+  <parameter id="test-node-title">COMP-0-Testnode</parameter>
+<parameter id="test-suite-master-url">https://zope:insecure@softinst43496.host.vifib.net/erp5/portal_task_distribution/1</parameter>
+</instance>
+3bis/ Supply and request http://git.erp5.org/gitweb/slapos.git/blob_plain/refs/tags/slapos-0.92:/software/kvm/software.cfg on a public node (so that vnc frontends are ok). "domain" parameter should be [ipv6] of partition. ipv4:4443 should be 6tunnelled to ipv6:4443 (Note: here, instead, I just hacked kvm_frontend to listen on ipv6).
+3ter/ Supply and request http://git.erp5.org/gitweb/slapos.git/blob_plain/HEAD:/software/apache-frontend/software.cfg, with any "domain" (it won't be used), on a public node (so that web frontends are ok)
+4/ On the ERP5 instance, create a project, a Task Distribution (in portal_task_distribution, type Scalability Task Distribution)
+5/ On the ERP5 instance, create a Test Suite, validate it
+
+Note: the slapos nodes are currently deployed using slapos-in-partition.
+Note: you have to manually kill -10 the erp5testnode process to start deployment of test because it doesn't know when SR installation is finished.
+Note: you have to manually run slapos-node-software --all on the slapos nodes if you are developping the SR you are testing.

slapos/resiliencytest/__init__.py

@@ -37,10 +37,6 @@ import traceback
 from erp5.util import taskdistribution
 from erp5.util.testnode import Utils
 
-MAX_INSTALLATION_TIME = 60 * 50
-MAX_TESTING_TIME = 60
-MAX_GETTING_CONNECTION_TIME = 60 * 5
-
 def importFrom(name):
   """
   Import a test suite module (in the suites module) and return it.
@@ -147,7 +143,7 @@ class ScalabilityLauncher(object):
     Return a ScalabilityTest with current running test case informations,
     or None if no test_case ready
     """
-    data = self.test_result.getNextTestCase()
+    data = self.test_result.getRunningTestCase()
     if data == None:
       return None
     decoded_data = Utils.deunicodeData(json.loads(

slapos/resiliencytest/suites/kvm.py

@@ -42,6 +42,10 @@ import urllib
 
 logger = logging.getLogger('KVMResiliencyTest')
 
+# Wait for 2 hours before renaming, so that replication of data is done
+# (~1GB of data to backup)
+SLEEP_TIME = 2 * 60 * 60
+
 def fetchMainInstanceIP(current_partition, software_release, instance_name):
   return current_partition.request(
       software_release=software_release,
@@ -105,7 +109,9 @@ def runTestSuite(server_url, key_file, cert_file,
   3/ Resilience is done, wait XX seconds
   4/ For each clone: do a takeover. Check that IPv6 of new main instance is different. Check, when doing a http request to the new VM that will fetch the stored random number, that the sent number is the same.
 
-  Note: disk image is a simple debian with the following python code running at boot:
+  Note: disk image is a simple debian with gunicorn and flask installed:
+    apt-get install python-setuptools; easy_install gunicorn flask
+  With the following python code running at boot in /root/number.py:
 
   import os
   
@@ -115,7 +121,7 @@ def runTestSuite(server_url, key_file, cert_file,
   storage = 'storage.txt'
   
   @app.route("/")
-  def greeting_list(): # 'cause they are several greetings, and plural is forbidden.
+  def greeting_list(): # 'cause there are several greetings, and plural is forbidden.
     return "Hello World"
   
   @app.route("/get")
@@ -124,13 +130,39 @@ def runTestSuite(server_url, key_file, cert_file,
   
   @app.route("/set")
   def set():
-    if os.path.exists(storage):
-      abort(503)
+    #if os.path.exists(storage):
+    #  abort(503)
     open(storage, 'w').write(request.args['key'])
     return "OK"
   
   if __name__ == "__main__":
     app.run(host='0.0.0.0', port=80)
+
+
+  Then create the boot script:
+  echo "cd /root; /usr/local/bin/gunicorn number:app -b 0.0.0.0:80 -D --error-logfile /root/error_log --access-logfile /root/access_log" > /etc/init.d/gunicorn-number
+  chmod +x /etc/init.d/gunicorn-number
+  update-rc.d gunicorn-number defaults
+
+
+  There also is a script that randomly generates I/O in /root/io.sh:
+
+  #!/bin/sh
+  # Randomly generates high I/O on disk. Goal is to write on disk so that
+  # it flushes at the same time that snapshot of disk image is done, to check if
+  # it doesn't corrupt image.
+  # Ayayo!
+  while [ 1 ]; do
+    dd if=/dev/urandom of=random count=2k
+    sync
+    sleep 0.2
+  done
+
+  Then create the boot script:
+  echo "/bin/sh /root/io.sh &" > /etc/init.d/io
+  chmod +x /etc/init.d/io
+  update-rc.d io defaults
+
   """
   slap = slapos.slap.slap()
   slap.initializeConnection(server_url, key_file, cert_file)
@@ -142,9 +174,6 @@ def runTestSuite(server_url, key_file, cert_file,
   ip = fetchMainInstanceIP(partition, software, kvm_rootinstance_name)
   logger.info('KVM IP is %s.' % ip)
 
-  key = setRandomKey(ip)
-  logger.info('Key set for test in current KVM: %s.' % key)
-
   # In resilient stack, main instance (example with KVM) is named "kvm0",
   # clones are named "kvm1", "kvm2", ...
   clone_count = int(total_instance_count) - 1
@@ -154,13 +183,17 @@ def runTestSuite(server_url, key_file, cert_file,
   # Test each clone
   while current_clone <= clone_count:
     logger.info('Testing kvm%s.' % current_clone)
-    # Wait for XX minutes so that replication is done
-    sleep_time = 60 * 15#2 * 60 * 60
-    logger.info('Sleeping for %s seconds.' % sleep_time)
-    time.sleep(sleep_time)
+
+    key = setRandomKey(ip)
+    logger.info('Key set for test in current KVM: %s.' % key)
+
+    logger.info('Sleeping for %s seconds.' % SLEEP_TIME)
+    time.sleep(SLEEP_TIME)
 
     # Make the clone instance takeover the main instance
     logger.info('Replacing main instance by clone instance...')
+    for i in range(0, 10):
+      try:
         takeover(
             server_url=server_url,
             key_file=key_file,
@@ -171,6 +204,13 @@ def runTestSuite(server_url, key_file, cert_file,
             namebase=namebase,
             winner_instance_suffix=str(current_clone),
         )
+        break
+      except: # SSLError
+        traceback.print_exc()
+        if i is 9:
+          raise
+        logger.warning('takeover failed. Retrying...')
+        time.sleep(10)
     logger.info('Done.')
 
     # Wait for the new IP (of old-clone new-main instance) to appear.

slapos/resiliencytest/suites/resiliencytestsuite.py

@@ -42,6 +42,7 @@ class ResiliencyTestSuite(object):
                computer_id, partition_id, software,
                namebase,
                root_instance_name,
+               sleep_time_between_test=600,
                total_instance_count="3"):
     self.server_url = server_url
     self.key_file = key_file
@@ -52,6 +53,7 @@ class ResiliencyTestSuite(object):
     self.namebase = namebase
     self.total_instance_count = total_instance_count
     self.root_instance_name = root_instance_name
+    self.sleep_time_between_test = sleep_time_between_test
 
     slap = slapos.slap.slap()
     slap.initializeConnection(server_url, key_file, cert_file)
@@ -151,12 +153,12 @@ class ResiliencyTestSuite(object):
     # Test each clone
     while current_clone <= clone_count:
       # Wait for XX minutes so that replication is done
-      sleep_time = 60 * 15#2 * 60 * 60
       self.logger.info('Sleeping for %s seconds before testing clone %s.' % (
-          sleep_time,
+          self.sleep_time_between_test,
           current_clone
       ))
-      time.sleep(sleep_time)
+      time.sleep(self.sleep_time_between_test)
+
       self._doTakeover(current_clone)
       self.logger.info('Testing %s%s instance.' % (self.namebase, current_clone))
       success = self.checkDataOnCloneInstance()

slapos/resiliencytest/suites/slaprunner.py

@@ -50,22 +50,19 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
 
     # Setup urllib2 with cookie support
     cookie_jar = cookielib.CookieJar()
-    self._opener_director = urllib2.build_opener(urllib2.HTTPCookieProcessor(cookie_jar))
+    self._opener_director = urllib2.build_opener(
+        urllib2.HTTPCookieProcessor(cookie_jar)
+    )
 
     ResiliencyTestSuite.__init__(
         self,
         server_url, key_file, cert_file,
         computer_id, partition_id, software,
         namebase,
-        slaprunner_rootinstance_name
+        slaprunner_rootinstance_name,
+        300
     )
 
-  def generateData(self):
-    self.slaprunner_password = ''.join(random.SystemRandom().sample(string.ascii_lowercase, 8))
-    self.slaprunner_user = 'slapos'
-    self.logger.info('Generated slaprunner user is: %s' % self.slaprunner_user)
-    self.logger.info('Generated slaprunner password is: %s' % self.slaprunner_password)
-
   def _connectToSlaprunner(self, resource, data=None):
     """
     Utility.
@@ -84,23 +81,92 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
 
   def _login(self):
     self.logger.debug('Logging in...')
-    self._connectToSlaprunner('doLogin', data='clogin=%s&cpwd=%s' % (self.slaprunner_user, self.slaprunner_password))
+    self._connectToSlaprunner('doLogin', data='clogin=%s&cpwd=%s' % (
+        self.slaprunner_user,
+        self.slaprunner_password)
+    )
 
   def _retrieveInstanceLogFile(self):
     """
-    Store the logfile (=data) of the instance, check it is not empty nor it is html.
+    Store the logfile (=data) of the instance, check it is not empty nor it is
+    html.
     """
-    data = self._connectToSlaprunner(resource='fileBrowser', data='opt=9&filename=log.log&dir=instance_root%252Fslappart0%252Fvar%252Flog%252F')
+    data = self._connectToSlaprunner(
+        resource='fileBrowser',
+        data='opt=9&filename=log.log&dir=instance_root%252Fslappart0%252Fvar%252Flog%252F'
+    )
     self.logger.info('Retrieved data are:\n%s' % data)
 
     if data.find('<') is not -1:
-      raise IOError('Could not retrieve logfile content: retrieved content is html.')
+      raise IOError(
+          'Could not retrieve logfile content: retrieved content is html.'
+      )
     if data.find('Could not load') is not -1:
-      raise IOError('Could not retrieve logfile content: server could not load the file.')
+      raise IOError(
+          'Could not retrieve logfile content: server could not load the file.'
+      )
     if data.find('Hello') is -1:
-      raise IOError('Could not retrieve logfile content: retrieve content does not match "Hello".')
+      raise IOError(
+          'Could not retrieve logfile content: retrieve content does not match "Hello".'
+      )
     return data
 
+  def _waitForSoftwareBuild(self):
+    while self._connectToSlaprunner(resource='slapgridResult', data='position=0&log=').find('"software": true') is not -1:
+      self.logger.info('Software release is still building. Sleeping...')
+      time.sleep(15)
+    self.logger.info('Software Release has been built / is no longer building.')
+
+  def _buildSoftwareRelease(self):
+    self.logger.info('Building the Software Release...')
+    try:
+      self._connectToSlaprunner(resource='runSoftwareProfile')
+    except (NotHttpOkException, urllib2.HTTPError):
+      # The nginx frontend might timeout before software release is finished.
+      pass
+    self._waitForSoftwareBuild()
+
+  def _deployInstance(self):
+    self.logger.info('Deploying instance...')
+    try:
+      self._connectToSlaprunner(resource='runInstanceProfile')
+    except (NotHttpOkException, urllib2.HTTPError):
+      # The nginx frontend might timeout before someftware release is finished.
+      pass
+    while self._connectToSlaprunner(resource='slapgridResult', data='position=0&log=').find('"instance": true') is not -1:
+      self.logger.info('Buildout is still running. Sleeping...')
+      time.sleep(15)
+    self.logger.info('Instance has been deployed.')
+
+  def _gitClone(self):
+    self.logger.debug('Doing git clone of git.erp5.org/repos/slapos.git...')
+    try:
+      self._connectToSlaprunner(
+          resource='cloneRepository',
+          data='repo=http://git.erp5.org/repos/slapos.git&name=workspace/slapos&email=slapos@slapos.org&user=slapos'
+      )
+    except (NotHttpOkException, urllib2.HTTPError):
+      # cloning can be very long.
+      # XXX: quite dirty way to check.
+      while self._connectToSlaprunner('getProjectStatus', data='project=workspace/slapos').find('On branch master') is -1:
+        self.logger.info('git-cloning ongoing, sleeping...')
+
+  def _openSoftwareRelease(self, software_name):
+    self.logger.debug('Opening %s software release...' % software_name)
+    self._connectToSlaprunner(
+        resource='setCurrentProject',
+        data='path=workspace/slapos/software/%s/' % software_name
+    )
+
+
+  def generateData(self):
+    self.slaprunner_password = ''.join(
+        random.SystemRandom().sample(string.ascii_lowercase, 8)
+    )
+    self.slaprunner_user = 'slapos'
+    self.logger.info('Generated slaprunner user is: %s' % self.slaprunner_user)
+    self.logger.info('Generated slaprunner password is: %s' % self.slaprunner_password)
+
   def pushDataOnMainInstance(self):
     """
     Create a dummy Software Release,
@@ -117,47 +183,26 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
     slaprunner_recovery_code = parameter_dict['password_recovery_code']
 
     self.logger.debug('Creating the slaprunner account...')
-    self._connectToSlaprunner(resource='configAccount', data='name=slapos&username=%s&email=slapos@slapos.org&password=%s&rcode=%s' % (self.slaprunner_user, self.slaprunner_password, slaprunner_recovery_code))
+    self._connectToSlaprunner(
+        resource='configAccount',
+        data='name=slapos&username=%s&email=slapos@slapos.org&password=%s&rcode=%s' % (
+            self.slaprunner_user,
+            self.slaprunner_password,
+            slaprunner_recovery_code
+        )
+    )
 
     self._login()
 
-    self.logger.debug('Opening hello-world software release from git...')
-    try:
-      self._connectToSlaprunner(resource='cloneRepository', data='repo=http://git.erp5.org/repos/slapos.git&name=workspace/slapos&email=slapos@slapos.org&user=slapos')
-    except (NotHttpOkException, urllib2.HTTPError):
-      # cloning can be very long.
-      # XXX: quite dirty way to check.
-      while self._connectToSlaprunner('getProjectStatus', data='project=workspace/slapos').find('On branch master') is -1:
-        self.logger.info('git-cloning ongoing, sleeping...')
-
+    self._gitClone()
     # XXX should be taken from parameter.
-    self._connectToSlaprunner(resource='setCurrentProject', data='path=workspace/slapos/software/helloworld/')
+    self._openSoftwareRelease('helloworld')
 
-    self.logger.info('Building the Software Release...')
-    try:
-      self._connectToSlaprunner(resource='runSoftwareProfile')
-    except (NotHttpOkException, urllib2.HTTPError):
-      # The nginx frontend might timeout before software release is finished.
-      pass
-    while self._connectToSlaprunner(resource='slapgridResult', data='position=0&log=').find('"software": true') is not -1:
-      self.logger.info('Buildout is still running. Sleeping...')
-      time.sleep(15)
-    self.logger.info('Software Release has been built.')
-
-    self.logger.info('Deploying instance...')
-    try:
-      self._connectToSlaprunner(resource='runInstanceProfile')
-    except (NotHttpOkException, urllib2.HTTPError):
-      # The nginx frontend might timeout before someftware release is finished.
-      pass
-    while self._connectToSlaprunner(resource='slapgridResult', data='position=0&log=').find('"instance": true') is not -1:
-      self.logger.info('Buildout is still running. Sleeping...')
-      time.sleep(15)
-    self.logger.info('Instance has been deployed.')
+    self._buildSoftwareRelease()
+    self._deployInstance()
 
     self.data = self._retrieveInstanceLogFile()
 
-
   def checkDataOnCloneInstance(self):
     """
     Check that:
@@ -173,6 +218,10 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
         old_parameter_value=old_slaprunner_backend_url
     )
     self._login()
+    self._waitForSoftwareBuild()
+    # XXX: in theory, it should be done automatically by slaprunner.
+    #      In practice, it is still too dangerous for ERP5 instances.
+    self._deployInstance()
     new_data = self._retrieveInstanceLogFile()
 
     if new_data == self.data:

slapos/runner/__init__.py

@@ -21,8 +21,7 @@ class Parser(OptionParser):
     """
     Initialize all possible options.
     """
-    OptionParser.__init__(self, usage=usage, version=version,
-                          option_list=[
+    option_list = [
       Option("-l", "--log_file",
              help="The path to the log file used by the script.",
              type=str),
@@ -38,7 +37,10 @@ class Parser(OptionParser):
              default=False,
              action="store_true",
              help="Debug mode."),
-    ])
+    ]
+
+    OptionParser.__init__(self, usage=usage, version=version,
+                          option_list=option_list)
 
   def check_args(self):
     """
@@ -50,6 +52,7 @@ class Parser(OptionParser):
 
     return options, args[0]
 
+
 class Config:
   def __init__(self):
     self.configuration_file_path = None
@@ -125,6 +128,7 @@ def run():
 
   sys.exit(return_code)
 
+
 def serve(config):
   from views import app
   from werkzeug.contrib.fixers import ProxyFix
@@ -134,7 +138,7 @@ def serve(config):
   app.config.update(
     software_log=config.software_root.rstrip('/') + '.log',
     instance_log=config.instance_root.rstrip('/') + '.log',
-    workspace = workdir,
+    workspace=workdir,
     software_link=software_link,
     instance_profile='instance.cfg',
     software_profile='software.cfg',

slapos/runner/decorators.py

@@ -11,4 +11,3 @@ def as_json(f):
   def inner(*args, **kwargs):
     return Response(json.dumps(f(*args, **kwargs)), mimetype='application/json')
   return inner
-

slapos/runner/fileBrowser.py

@@ -15,7 +15,7 @@ from slapos.runner.utils import realpath, tail, isText
 
 
 class FileBrowser(object):
-  """This class contain all bases function for file browser"""
+  """This class contains all base functions for file browser"""
 
   def __init__(self, config):
     self.config = config
@@ -31,14 +31,14 @@ class FileBrowser(object):
     html = 'var gsdirs = [], gsfiles = [];'
 
     dir = urllib.unquote(dir)
-    # 'dir' is used below. XXX should not shadow a builtin name
+    # XXX-Marco 'dir' and 'all' should not shadow builtin names
     realdir = realpath(self.config, dir)
     if not realdir:
       raise NameError('Could not load directory %s: Permission denied' % dir)
 
     ldir = sorted(os.listdir(realdir), key=str.lower)
     for f in ldir:
-      if f.startswith('.') and not all: #do not display this file/folder
+      if f.startswith('.') and not all:  # do not display this file/folder
         continue
       ff = os.path.join(dir, f)
       realfile = os.path.join(realdir, f)
@@ -53,25 +53,20 @@ class FileBrowser(object):
           ext = "unknow"
         else:
           ext = str.lower(ext)
-        html += 'gsfiles.push(new gsItem("1", "' + f + '", "' + \
-                  ff + '", "' + str(size) + '", "' + md5sum + \
-                  '", "' + ext + '", "' + mdate + '"));'
+        html += 'gsfiles.push(new gsItem("1", "%s", "%s", "%s", "%s", "%s", "%s"));' % (f, ff, size, md5sum, ext, mdate)
       else:
-        html += 'gsdirs.push(new gsItem("2", "' + f + '", "' + \
-                  ff + '", "0", "' + md5sum + '", "dir", "' + mdate + '"));'
+        html += 'gsdirs.push(new gsItem("2", "%s", "%s", "0", "%s", "dir", "%s"));' % (f, ff, md5sum, mdate)
     return html
 
-
   def makeDirectory(self, dir, filename):
     """Create a directory"""
     realdir = self._realdir(dir)
     folder = os.path.join(realdir, filename)
     if not os.path.exists(folder):
       os.mkdir(folder, 0744)
-      return '{result: \'1\'}'
+      return "{result: '1'}"
     else:
-      return '{result: \'0\'}'
-
+      return "{result: '0'}"
 
   def makeFile(self, dir, filename):
     """Create a file in a directory dir taken"""
@@ -79,36 +74,39 @@ class FileBrowser(object):
     fout = os.path.join(realdir, filename)
     if not os.path.exists(fout):
       open(fout, 'w')
-      return 'var responce = {result: \'1\'}'
+      return "var responce = {result: '1'}"
     else:
-      return '{result: \'0\'}'
+      return "{result: '0'}"
 
   def deleteItem(self, dir, files):
     """Delete a list of files or directories"""
+    # XXX-Marco do not shadow 'dir'
     realdir = self._realdir(dir)
     lfiles = urllib.unquote(files).split(',,,')
     try:
+      # XXX-Marco do not shadow 'file'
       for file in lfiles:
         file = os.path.join(realdir, file)
         if not os.path.exists(file):
-          continue #silent skip file....
+          continue  # silent skip file....
         details = file.split('/')
         last = details[-1]
         if last and last.startswith('.'):
-          continue #cannot delete this file/directory, to prevent security
+          continue  # cannot delete this file/directory, to prevent security
         if os.path.isdir(file):
           shutil.rmtree(file)
         else:
           os.unlink(file)
     except Exception as e:
       return str(e)
-    return '{result: \'1\'}'
+    return "{result: '1'}"
 
   def copyItem(self, dir, files, del_source=False):
     """Copy a list of files or directory to dir"""
     realdir = self._realdir(dir)
     lfiles = urllib.unquote(files).split(',,,')
     try:
+      # XXX-Marco do not shadow 'file'
       for file in lfiles:
         realfile = realpath(self.config, file)
         if not realfile:
@@ -117,7 +115,7 @@ class FileBrowser(object):
         details = realfile.split('/')
         dest = os.path.join(realdir, details[-1])
         if os.path.exists(dest):
-          raise NameError('NOT ALLOWED OPERATION : File or directory already exist')
+          raise NameError('NOT ALLOWED OPERATION : File or directory already exists')
         if os.path.isdir(realfile):
           shutil.copytree(realfile, dest)
           if del_source:
@@ -128,7 +126,7 @@ class FileBrowser(object):
             os.unlink(realfile)
     except Exception as e:
       return str(e)
-    return '{result: \'1\'}'
+    return "{result: '1'}"
 
   def rename(self, dir, filename, newfilename):
     """Rename file or directory to dir/filename"""
@@ -139,8 +137,8 @@ class FileBrowser(object):
     tofile = os.path.join(realdir, newfilename)
     if not os.path.exists(tofile):
       os.rename(realfile, tofile)
-      return '{result: \'1\'}'
-    raise NameError('NOT ALLOWED OPERATION : File or directory already exist')
+      return "{result: '1'}"
+    raise NameError('NOT ALLOWED OPERATION : File or directory already exists')
 
   def copyAsFile(self, dir, filename, newfilename):
     """Copy file or directory to dir/filename"""
@@ -148,21 +146,21 @@ class FileBrowser(object):
     fromfile = os.path.join(realdir, filename)
     tofile = os.path.join(realdir, newfilename)
     if not os.path.exists(fromfile):
-      raise NameError('NOT ALLOWED OPERATION : File or directory not exist')
+      raise NameError('NOT ALLOWED OPERATION : File or directory does not exist')
     if not os.path.exists(tofile):
       shutil.copy(fromfile, tofile)
-      return '{result: \'1\'}'
-    raise NameError('NOT ALLOWED OPERATION : File or directory already exist')
+      return "{result: '1'}"
+    raise NameError('NOT ALLOWED OPERATION : File or directory already exists')
 
   def uploadFile(self, dir, files):
-    """Upload a list of file in directory dir"""
+    """Upload a list of files in directory dir"""
     realdir = self._realdir(dir)
     for file in files:
       if files[file]:
         filename = werkzeug.secure_filename(files[file].filename)
         if not os.path.exists(os.path.join(dir, filename)):
           files[file].save(os.path.join(realdir, filename))
-    return '{result: \'1\'}'
+    return "{result: '1'}"
 
   def downloadFile(self, dir, filename):
     """Download file dir/filename"""
@@ -179,7 +177,7 @@ class FileBrowser(object):
     tozip = os.path.join(realdir, newfilename)
     fromzip = os.path.join(realdir, filename)
     if not os.path.exists(fromzip):
-      raise NameError('NOT ALLOWED OPERATION : File or directory not exist')
+      raise NameError('NOT ALLOWED OPERATION : File or directory does not exist')
     if not os.path.exists(tozip):
       zip = zipfile.ZipFile(tozip, 'w', zipfile.ZIP_DEFLATED)
       if os.path.isdir(fromzip):
@@ -191,8 +189,8 @@ class FileBrowser(object):
       else:
         zip.write(fromzip)
       zip.close()
-      return '{result: \'1\'}'
-    raise NameError('NOT ALLOWED OPERATION : File or directory already exist')
+      return "{result: '1'}"
+    raise NameError('NOT ALLOWED OPERATION : File or directory already exists')
 
   def unzipFile(self, dir, filename, newfilename):
     """Extract a zipped archive"""
@@ -200,7 +198,7 @@ class FileBrowser(object):
     target = os.path.join(realdir, newfilename)
     archive = os.path.join(realdir, filename)
     if not os.path.exists(archive):
-      raise NameError('NOT ALLOWED OPERATION : File or directory not exist')
+      raise NameError('NOT ALLOWED OPERATION : File or directory does not exist')
     if not os.path.exists(target):
       zip = zipfile.ZipFile(archive)
       #member = zip.namelist()
@@ -209,8 +207,8 @@ class FileBrowser(object):
       #  zip.extractall(target)
       #else:
       #  zip.extract(member[0], newfilename)
-      return '{result: \'1\'}'
-    raise NameError('NOT ALLOWED OPERATION : File or directory already exist')
+      return "{result: '1'}"
+    raise NameError('NOT ALLOWED OPERATION : File or directory already exists')
 
   def readFile(self, dir, filename, truncate=False):
     """Read file dir/filename and return content"""
@@ -221,6 +219,6 @@ class FileBrowser(object):
     if not isText(realfile):
       return "FILE ERROR: Cannot display binary file, please open a text file only!"
     if not truncate:
-      return open(realfile, 'r').read()
+      return open(realfile).read()
     else:
-      return tail(open(realfile, 'r'), 0)
+      return tail(open(realfile), 0)

slapos/runner/gittools.py

@@ -12,12 +12,12 @@ from flask import jsonify
 
 
 def cloneRepo(data):
-  """Clonne a repository
+  """Clone a repository
   Args:
-    data: a dictionnary of parameters to use:
+    data: a dictionary of parameters to use:
       data['path'] is the path of the new project
       data['repo'] is the url of the repository to be cloned
-      data['email'] is the user email
+      data['email'] is the user's email
       data['user'] is the name of the user
   Returns:
     a jsonify data"""
@@ -29,7 +29,7 @@ def cloneRepo(data):
   json = ""
   try:
     if os.path.exists(workDir) and len(os.listdir(workDir)) < 2:
-      shutil.rmtree(workDir) #delete useless files
+      shutil.rmtree(workDir)  # delete useless files
     repo = Repo.clone_from(data["repo"], workDir)
     config_writer = repo.config_writer()
     config_writer.add_section("user")
@@ -42,10 +42,11 @@ def cloneRepo(data):
     json = safeResult(str(e))
   return jsonify(code=code, result=json)
 
+
 def gitStatus(project):
   """Run git status and return status of specified project folder
   Args:
-    project: path of the projet ti get status
+    project: path of the projet to get status
   Returns:
     a parsed string that contains the result of git status"""
   code = 0
@@ -61,6 +62,7 @@ def gitStatus(project):
     json = safeResult(str(e))
   return jsonify(code=code, result=json, branch=branch, dirty=isdirty)
 
+
 def switchBranch(project, name):
   """Switch a git branch
   Args:
@@ -83,6 +85,7 @@ def switchBranch(project, name):
     json = safeResult(str(e))
   return jsonify(code=code, result=json)
 
+
 def addBranch(project, name, onlyCheckout=False):
   """Add new git branch to the repository
   Args:
@@ -105,6 +108,7 @@ def addBranch(project, name, onlyCheckout=False):
     json = safeResult(str(e))
   return jsonify(code=code, result=json)
 
+
 def getDiff(project):
   """Get git diff for the specified project directory"""
   result = ""
@@ -117,44 +121,40 @@ def getDiff(project):
     result = safeResult(str(e))
   return result
 
-def gitCommit(project, msg):
-  """Commit changes for the specified repository
+
+def gitPush(project, msg):
+  """Commit and Push changes for the specified repository
   Args:
     project: directory of the local repository
     msg: commit message"""
-  code = 1
+  code = 0
   json = ""
+  undo_commit = False
+  try:
     repo = Repo(project)
     if repo.is_dirty:
       git = repo.git
+      current_branch = repo.active_branch.name
       #add file to be commited
       files = repo.untracked_files
       for f in files:
         git.add(f)
       #Commit all modified and untracked files
       git.commit('-a', '-m', msg)
-  else:
-    json = "Nothing to be commited"
-  return jsonify(code=code, result=json)
-
-def gitPush(project):
-  """Push changes for the specified repository
-  Args:
-    project: directory of the local repository
-    msg: commit message"""
-  code = 0
-  json = ""
-  repo = Repo(project)
-  try:
-    git = repo.git
+      undo_commit = True
       #push changes to repo
-    current_branch = repo.active_branch.name
       git.push('origin', current_branch)
       code = 1
+    else:
+      json = "Nothing to commit"
+      code = 1
   except Exception as e:
+    if undo_commit:
+      git.reset("HEAD~")  # undo previous commit
     json = safeResult(str(e))
   return jsonify(code=code, result=json)
 
+
 def gitPull(project):
   result = ""
   code = 0
@@ -167,6 +167,7 @@ def gitPull(project):
     result = safeResult(str(e))
   return jsonify(code=code, result=result)
 
+
 def safeResult(result):
   """Parse string and remove credential of the user"""
   regex = re.compile("(https:\/\/)([\w\d\._-]+:[\w\d\._-]+)\@([\S]+\s)", re.VERBOSE)

slapos/runner/process.py

@@ -9,7 +9,7 @@ SLAPRUNNER_PROCESS_LIST = []
 
 class Popen(subprocess.Popen):
   """
-  Extension of Popen to launch and kill process in a clean way
+  Extension of Popen to launch and kill processes in a clean way
   """
   def __init__(self, *args, **kwargs):
     """
@@ -28,7 +28,7 @@ class Popen(subprocess.Popen):
 
   def kill(self, sig=signal.SIGTERM, recursive=False):
     """
-    Kill process and all its descendant if recursive
+    Kill process and all its descendants if recursive
     """
     if self.poll() is None:
       if recursive:
@@ -83,7 +83,7 @@ def isRunning(name):
 
 def killRunningProcess(name, recursive=False):
   """
-  Kill all process with name
+  Kill all processes with a given name
   """
   for process in SLAPRUNNER_PROCESS_LIST:
     if process.name == name:
@@ -92,7 +92,7 @@ def killRunningProcess(name, recursive=False):
 
 def handler(sig, frame):
   """
-  Signal handler to kill all process
+  Signal handler to kill all processes
   """
   pid = os.getpid()
   os.kill(-pid, sig)

slapos/runner/runnertest.py

@@ -5,12 +5,12 @@
 import argparse
 import ConfigParser
 import datetime
+import hashlib
 import json
 import os
 import shutil
 import time
 import unittest
-import hashlib
 
 from slapos.runner.utils import (getProfilePath, getSession, isInstanceRunning,
                                  isSoftwareRunning, startProxy)
@@ -18,6 +18,7 @@ from slapos.runner.process import killRunningProcess, isRunning
 from slapos.runner import views
 import slapos.slap
 
+
 #Helpers
 def loadJson(response):
   return json.loads(response.data)
@@ -48,6 +49,7 @@ class Config:
         if not getattr(self, key, None):
           setattr(self, key, configuration_dict[key])
 
+
 class SlaprunnerTestCase(unittest.TestCase):
 
   def setUp(self):
@@ -57,8 +59,8 @@ class SlaprunnerTestCase(unittest.TestCase):
     self.updateUser = ["newslapuser", "newslappwd", "slaprunner@nexedi.com", "SlapOS web runner"]
     self.rcode = "41bf2657"
     self.repo = 'http://git.erp5.org/repos/slapos.git'
-    self.software = "workspace/slapos/software/" #relative directory fo SR
-    self.project = 'slapos' #Default project name
+    self.software = "workspace/slapos/software/"  # relative directory fo SR
+    self.project = 'slapos'  # Default project name
     self.template = 'template.cfg'
     self.partitionPrefix = 'slappart'
     self.slaposBuildout = "1.6.0-dev-SlapOS-010"
@@ -77,7 +79,7 @@ class SlaprunnerTestCase(unittest.TestCase):
     views.app.config.update(
       software_log=config.software_root.rstrip('/') + '.log',
       instance_log=config.instance_root.rstrip('/') + '.log',
-      workspace = workdir,
+      workspace=workdir,
       software_link=software_link,
       instance_profile='instance.cfg',
       software_profile='software.cfg',
@@ -87,9 +89,8 @@ class SlaprunnerTestCase(unittest.TestCase):
     self.app = views.app.test_client()
     self.app.config = views.app.config
     #Create password recover code
-    rpwd = open(os.path.join(views.app.config['etc_dir'], '.rcode'), 'w')
+    with open(os.path.join(views.app.config['etc_dir'], '.rcode'), 'w') as rpwd:
       rpwd.write(self.rcode)
-    rpwd.close()
 
   def tearDown(self):
     """Remove all test data"""
@@ -117,20 +118,24 @@ class SlaprunnerTestCase(unittest.TestCase):
 
   def configAccount(self, username, password, email, name, rcode):
     """Helper for configAccount"""
-    return self.app.post('/configAccount', data=dict(
+    return self.app.post('/configAccount',
+                         data=dict(
                            username=username,
                            password=password,
                            email=email,
                            name=name,
                            rcode=rcode
-          ), follow_redirects=True)
+                         ),
+                         follow_redirects=True)
 
   def login(self, username, password):
     """Helper for Login method"""
-    return self.app.post('/doLogin', data=dict(
+    return self.app.post('/doLogin',
+                         data=dict(
                            clogin=username,
                            cpwd=password
-          ), follow_redirects=True)
+                         ),
+                         follow_redirects=True)
 
   def setAccount(self):
     """Initialize user account and log user in"""
@@ -146,13 +151,15 @@ class SlaprunnerTestCase(unittest.TestCase):
 
   def updateAccount(self, newaccount, rcode):
     """Helper for update user account data"""
-    return self.app.post('/updateAccount', data=dict(
+    return self.app.post('/updateAccount',
+                         data=dict(
                            username=newaccount[0],
                            password=newaccount[1],
                            email=newaccount[2],
                            name=newaccount[3],
                            rcode=rcode
-          ), follow_redirects=True)
+                         ),
+                         follow_redirects=True)
 
   def getCurrentSR(self):
    return getProfilePath(self.app.config['etc_dir'],
@@ -167,7 +174,7 @@ class SlaprunnerTestCase(unittest.TestCase):
     self.assertEqual(proxy, status)
 
   def setupProjectFolder(self, withSoftware=False):
-    """Helper for create a project folder as for slapos.git"""
+    """Helper to create a project folder as for slapos.git"""
     base = os.path.join(self.app.config['workspace'], 'slapos')
     software = os.path.join(base, 'software')
     os.mkdir(base)
@@ -186,10 +193,11 @@ class SlaprunnerTestCase(unittest.TestCase):
                           'w').write(sr)
 
   def setupSoftwareFolder(self):
-    """Helper for setup compiled software release dir"""
+    """Helper to setup compiled software release dir"""
     self.setupProjectFolder(withSoftware=True)
     md5 = hashlib.md5(os.path.join(self.app.config['workspace'],
-        "slapos/software/slaprunner-test", self.app.config['software_profile'])
+                                   "slapos/software/slaprunner-test",
+                                   self.app.config['software_profile'])
                       ).hexdigest()
     base = os.path.join(self.app.config['software_root'], md5)
     template = os.path.join(base, self.template)
@@ -208,10 +216,9 @@ class SlaprunnerTestCase(unittest.TestCase):
     """Kill slapproxy process"""
     killRunningProcess('slapproxy', recursive=True)
 
-
   #Begin test case here
   def test_wrong_login(self):
-    """Test Login user before create session. This should return error value"""
+    """Test Login user before create session. This should return an error value"""
     response = self.login(self.users[0], self.users[1])
     #redirect to config account page
     assert "<h2 class='title'>Your personal information</h2><br/>" in response.data
@@ -242,7 +249,7 @@ class SlaprunnerTestCase(unittest.TestCase):
     assert "<h2>Login to Slapos Web Runner</h2>" in result.data
 
   def test_updateAccount(self):
-    """test Update accound, this need user to loging in"""
+    """test Update accound, this needs the user to log in"""
     self.setAccount()
     response = loadJson(self.updateAccount(self.updateUser, self.rcode))
     self.assertEqual(response['code'], 1)
@@ -266,25 +273,32 @@ class SlaprunnerTestCase(unittest.TestCase):
     """Start scenario 1 for deploying SR: Clone a project from git repository"""
     self.setAccount()
     folder = 'workspace/' + self.project
-    data = {"repo":self.repo, "user":'Slaprunner test',
-          "email":'slaprunner@nexedi.com', "name":folder}
+    data = {
+      'repo': self.repo,
+      'user': 'Slaprunner test',
+      'email': 'slaprunner@nexedi.com',
+      'name': folder
+    }
     response = loadJson(self.app.post('/cloneRepository', data=data,
                     follow_redirects=True))
     self.assertEqual(response['result'], "")
-    #Get realpath of create project
+    # Get realpath of create project
     path_data = dict(file=folder)
     response = loadJson(self.app.post('/getPath', data=path_data,
                     follow_redirects=True))
     self.assertEqual(response['code'], 1)
     realFolder = response['result'].split('#')[0]
     #Check git configuration
-    config = open(os.path.join(realFolder, '.git/config'), 'r').read()
+    config = open(os.path.join(realFolder, '.git/config')).read()
     assert "slaprunner@nexedi.com" in config and "Slaprunner test" in config
-    #Checkout to slaprunner branch, this supose that branch slaprunner exit
-    response = loadJson(self.app.post('/newBranch', data=dict(
+
+    # Checkout to slaprunner branch, this supposes that branch slaprunner exit
+    response = loadJson(self.app.post('/newBranch',
+                                      data=dict(
                                         project=folder,
                                         create='0',
-                    name='slaprunner'),
+                                        name='slaprunner'
+                                      ),
                                       follow_redirects=True))
     self.assertEqual(response['result'], "")
     self.logout()
@@ -308,7 +322,7 @@ class SlaprunnerTestCase(unittest.TestCase):
     self.test_cloneProject()
     #Login
     self.login(self.users[0], self.users[1])
-    software = os.path.join(self.software, 'drupal') #Drupal SR must exist in SR folder
+    software = os.path.join(self.software, 'drupal')  # Drupal SR must exist in SR folder
     response = loadJson(self.app.post('/setCurrentProject',
                                       data=dict(path=software),
                                       follow_redirects=True))
@@ -317,8 +331,9 @@ class SlaprunnerTestCase(unittest.TestCase):
     assert software in currentSR
     self.assertFalse(isInstanceRunning(self.app.config))
     self.assertFalse(isSoftwareRunning(self.app.config))
-    #Slapproxy process is supose to be started
-    #newSoftware = os.path.join(self.software, 'slaprunner-test')
+
+    # Slapproxy process is supposed to be started
+    # newSoftware = os.path.join(self.software, 'slaprunner-test')
     self.proxyStatus(True)
     self.stopSlapproxy()
     self.logout()
@@ -344,15 +359,16 @@ class SlaprunnerTestCase(unittest.TestCase):
                                                 content=softwareRelease),
                                       follow_redirects=True))
     self.assertEqual(response['result'], "")
-    #Compile software and wait until slapgrid it end
-    #this is supose to use curent SR
+
+    # Compile software and wait until slapgrid ends
+    # this is supposed to use current SR
     response = loadJson(self.app.post('/runSoftwareProfile',
                                       data=dict(),
                                       follow_redirects=True))
     self.assertTrue(response['result'])
     self.assertTrue(os.path.exists(self.app.config['software_root']))
     self.assertTrue(os.path.exists(self.app.config['software_log']))
-    assert "test-application" in open(self.app.config['software_log'], 'r').read()
+    assert "test-application" in open(self.app.config['software_log']).read()
     sr_dir = os.listdir(self.app.config['software_root'])
     self.assertEqual(len(sr_dir), 1)
     createdFile = os.path.join(self.app.config['software_root'], sr_dir[0],
@@ -391,8 +407,8 @@ class SlaprunnerTestCase(unittest.TestCase):
     self.assertNotEqual(partitionList, [])
     #Assume that the requested partition is partition 0
     slapParameterDict = partitionList[0].getInstanceParameterDict()
-    self.assertTrue(slapParameterDict.has_key('appname'))
-    self.assertTrue(slapParameterDict.has_key('cacountry'))
+    self.assertTrue('appname' in slapParameterDict)
+    self.assertTrue('cacountry' in slapParameterDict)
     self.assertEqual(slapParameterDict['appname'], 'slaprunnerTest')
     self.assertEqual(slapParameterDict['cacountry'], 'France')
     self.assertEqual(slapParameterDict['slap_software_type'], 'production')
@@ -422,7 +438,7 @@ class SlaprunnerTestCase(unittest.TestCase):
                                       follow_redirects=True))
     self.assertTrue(response['result'])
     #Check that all partitions has been created
-    assert "create-file" in open(self.app.config['instance_log'], 'r').read()
+    assert "create-file" in open(self.app.config['instance_log']).read()
     instanceDir = os.listdir(self.app.config['instance_root'])
     for num in range(int(self.app.config['partition_amount'])):
       partition = os.path.join(self.app.config['instance_root'],
@@ -439,6 +455,7 @@ class SlaprunnerTestCase(unittest.TestCase):
     self.stopSlapproxy()
     self.logout()
 
+
 def main():
   # Empty parser for now - so that erp5testnode is happy when doing --help
   parser = argparse.ArgumentParser()

slapos/runner/static/js/scripts/cookies.js

@@ -37,7 +37,7 @@ function deleteCookie(name, path, domain) {
 
 function setCookie(name, value, expires, path, domain, secure) {
     "use strict";
-    if (getCookie(name) !== null){
+    if (getCookie(name) !== null) {
         deleteCookie(name);
     }
     if (!expires) {

slapos/runner/static/js/scripts/login.js

@@ -32,8 +32,14 @@ $(document).ready(function () {
                 $("#error").Popup(data.result, {type: 'alert', duration: 3000});
             }
         })
-            .error(function () {
-                $("#error").Popup("Cannot send your account identifier please try again!!",
+            .error(function (response) {
+                if (response && response.status === 401) {
+                    $("#error").Popup('Login and/or password is incorrect.',
+                                      {type: 'alert', duration: 3000}
+                    );
+                    return
+                }
+                $("#error").Popup("Cannot send your account identifier",
                                   {type: 'alert', duration: 3000});
             })
             .complete(function () {

slapos/runner/static/js/scripts/process.js

@@ -44,7 +44,7 @@ function clearAll(setStop) {
     running = setStop;
 }
 
-function removeFirstLog(){
+function removeFirstLog() {
     "use strict";
     currentLogSize -= parseInt($("#salpgridLog p:first-child").attr('rel'), 10);
     $("#salpgridLog p:first-child").remove();
@@ -52,16 +52,16 @@ function removeFirstLog(){
 
 function getRunningState() {
     "use strict";
-    var size = 0;
-    var log_info = "";
-    var param = {
+    var size = 0,
+        log_info = "",
+        param = {
             position: logReadingPosition,
             log: (processState !== "Checking" && openedlogpage === processType.toLowerCase()) ? openedlogpage : ""
         },
         jqxhr = $.post(url, param, function (data) {
             setRunningState(data);
             size = data.content.position - logReadingPosition;
-            if (logReadingPosition !== 0 && data.content.truncated){
+            if (logReadingPosition !== 0 && data.content.truncated) {
                 log_info = "<p  class='info' rel='0'>SLAPRUNNER INFO: SLAPGRID-LOG HAS BEEN TRUNCATED HERE. To see full log reload your log page</p>";
             }
             logReadingPosition = data.content.position;
@@ -78,18 +78,16 @@ function getRunningState() {
             }
             processState = running ? "Running" : "Stopped";
             currentLogSize += parseInt(size, 10);
-            if (currentLogSize > maxLogSize){
+            if (currentLogSize > maxLogSize) {
                 //Remove the first element into log div
                 removeFirstLog();
-                if (currentLogSize > maxLogSize){
+                if (currentLogSize > maxLogSize) {
                     removeFirstLog(); //in cas of previous <p/> size is 0
                 }
             }
-        })
-        .error(function () {
+        }).error(function () {
             clearAll(false);
-        })
-        .complete(function () {
+        }).complete(function () {
             if (running) {
                 setTimeout(function () {
                     getRunningState();

slapos/runner/static/js/scripts/softwareFolder.js

@@ -19,11 +19,10 @@ $(document).ready(function () {
         send = false,
         edit = false,
         selection = "",
-        edit_status = "";
-
-    var base_path = function() {
+        edit_status = "",
+        base_path = function () {
             return softwareDisplay ? projectDir : currentProject;
-    }
+        };
 
     function setEditMode(file) {
         var i,

slapos/runner/static/js/scripts/workspace.js

@@ -6,5 +6,5 @@
 $(document).ready(function () {
     "use strict";
 
-    $('#fileNavigator').gsFileManager({script: $SCRIPT_ROOT + "/fileBrowser", root:'workspace/'});
+    $('#fileNavigator').gsFileManager({script: $SCRIPT_ROOT + "/fileBrowser", root: 'workspace/'});
 });

slapos/runner/utils.py

@@ -2,13 +2,10 @@
 # vim: set et sts=2:
 # pylint: disable-msg=W0311,C0301,C0103,C0111,W0141,W0142
 
-
-import md5
 import logging
+import md5
 import multiprocessing
 import re
-from slapos.runner.process import Popen, isRunning, killRunningProcess
-from slapos.htpasswd import HtpasswdFile
 import shutil
 import os
 import time
@@ -18,6 +15,8 @@ from xml.dom import minidom
 import xml_marshaller
 from flask import jsonify
 
+from slapos.runner.process import Popen, isRunning, killRunningProcess
+from slapos.htpasswd import HtpasswdFile
 import slapos.slap
 
 # Setup default flask (werkzeug) parser
@@ -33,25 +32,22 @@ html_escape_table = {
   "<": "&lt;",
 }
 
+
 def html_escape(text):
   """Produce entities within text."""
   return "".join(html_escape_table.get(c, c) for c in text)
 
+
 def getSession(config):
   """
   Get the session data of current user.
   Returns:
-    a list of user information or False if fail to read data.
+    a list of user information or None if the file does not exist.
   """
   user_path = os.path.join(config['etc_dir'], '.users')
-  user = ""
   if os.path.exists(user_path):
-    f = open(user_path, 'r')
-    user = f.read().split(';')
-    f.close()
-  if type(user) == type(""):
-    return False
-  return user
+    return open(user_path).read().split(';')
+
 
 def saveSession(config, account):
   """
@@ -71,11 +67,9 @@ def saveSession(config, account):
   backup = False
   try:
     if os.path.exists(user):
-      f = open(user, 'r')
       #backup previous data
-      data = f.read()
-      open(user+'.back', 'w').write(data)
-      f.close()
+      data = open(user).read()
+      open('%s.back' % user, 'w').write(data)
       backup = True
       if not account[1]:
         account[1] = data.split(';')[1]
@@ -83,7 +77,7 @@ def saveSession(config, account):
     open(user, 'w').write((';'.join(account)).encode("utf-8"))
     # Htpasswd file for cloud9
     # XXX Cedric Le N order of account list values suppose to be fixed
-    # Remove former file to avoid aoutdated accounts
+    # Remove former file to avoid outdated accounts
     if os.path.exists(htpasswdfile):
       os.remove(htpasswdfile)
     passwd = HtpasswdFile(htpasswdfile, create=True)
@@ -94,11 +88,12 @@ def saveSession(config, account):
     try:
       if backup:
         os.remove(user)
-        os.rename(user+'.back', user)
+        os.rename('%s.back' % user, user)
     except:
       pass
     return str(e)
 
+
 def getCurrentSoftwareReleaseProfile(config):
   """
   Returns used Software Release profile as a string.
@@ -111,6 +106,7 @@ def getCurrentSoftwareReleaseProfile(config):
   except:
     return False
 
+
 def requestInstance(config, software_type=None):
   """
   Request the main instance of our environment
@@ -120,7 +116,7 @@ def requestInstance(config, software_type=None):
     # Write it to conf file for later use
     open(software_type_path, 'w').write(software_type)
   elif os.path.exists(software_type_path):
-    software_type = open(software_type_path, 'r').read()
+    software_type = open(software_type_path).read()
   else:
     software_type = 'default'
 
@@ -131,7 +127,7 @@ def requestInstance(config, software_type=None):
   param_path = os.path.join(config['etc_dir'], ".parameter.xml")
   xml_result = readParameters(param_path)
   partition_parameter_kw = None
-  if type(xml_result) != type('') and xml_result.has_key('instance'):
+  if type(xml_result) != type('') and 'instance' in xml_result:
     partition_parameter_kw = xml_result['instance']
 
   return slap.registerOpenOrder().request(
@@ -143,6 +139,7 @@ def requestInstance(config, software_type=None):
       state=None,
       shared=False)
 
+
 def updateProxy(config):
   """
   Configure Slapos Node computer and partitions.
@@ -163,25 +160,32 @@ def updateProxy(config):
     'netmask': '255.255.255.255',
     'partition_list': [],
     'reference': config['computer_id'],
- 'software_root': config['software_root']}
+    'software_root': config['software_root']
+  }
+
   for i in xrange(0, int(config['partition_amount'])):
     partition_reference = '%s%s' % (prefix, i)
     partition_path = os.path.join(config['instance_root'], partition_reference)
     if not os.path.exists(partition_path):
       os.mkdir(partition_path)
     os.chmod(partition_path, 0750)
-    slap_config['partition_list'].append({'address_list': [{'addr': config['ipv4_address'],
-                                       'netmask': '255.255.255.255'},
-                                      {'addr': config['ipv6_address'],
-                                       'netmask': 'ffff:ffff:ffff::'},
+    slap_config['partition_list'].append({
+                                           'address_list': [
+                                              {
+                                                'addr': config['ipv4_address'],
+                                                'netmask': '255.255.255.255'
+                                              }, {
+                                                'addr': config['ipv6_address'],
+                                                'netmask': 'ffff:ffff:ffff::'
+                                              },
                                            ],
                                            'path': partition_path,
                                            'reference': partition_reference,
-                     'tap': {'name': partition_reference},
-                     })
+                                           'tap': {'name': partition_reference}})
   computer.updateConfiguration(xml_marshaller.xml_marshaller.dumps(slap_config))
   return True
 
+
 def updateInstanceParameter(config, software_type=None):
   """
   Reconfigure Slapproxy to re-deploy current Software Instance with parameters.
@@ -193,9 +197,12 @@ def updateInstanceParameter(config, software_type=None):
   if not (updateProxy(config) and requestInstance(config, software_type)):
     return False
 
+
 def startProxy(config):
   """Start Slapproxy server"""
-  if not isRunning('slapproxy'):
+  if isRunning('slapproxy'):
+    return
+
   log = os.path.join(config['log_dir'], 'slapproxy.log')
   Popen([config['slapproxy'], '--log_file', log,
          config['configuration_file_path']],
@@ -210,15 +217,17 @@ def stopProxy(config):
 
 
 def removeProxyDb(config):
-  """Remove Slapproxy database, this is use to initialize proxy for example when
+  """Remove Slapproxy database, this is used to initialize proxy for example when
     configuring new Software Release"""
   if os.path.exists(config['database_uri']):
     os.unlink(config['database_uri'])
 
+
 def isSoftwareRunning(config=None):
   """
     Return True if slapgrid-sr is still running and false if slapgrid if not
   """
+  # XXX-Marco what is 'config' for?
   return isRunning('slapgrid-sr')
 
 
@@ -227,8 +236,10 @@ def runSoftwareWithLock(config):
     Use Slapgrid to compile current Software Release and wait until
     compilation is done
   """
+  if isSoftwareRunning():
+    return False
+
   slapgrid_pid = os.path.join(config['run_dir'], 'slapgrid-sr.pid')
-  if not isSoftwareRunning():
   if not os.path.exists(config['software_root']):
     os.mkdir(config['software_root'])
   stopProxy(config)
@@ -238,6 +249,7 @@ def runSoftwareWithLock(config):
   if not updateProxy(config):
     return False
   # Accelerate compilation by setting make -jX
+  # XXX-Marco can have issues with implicit dependencies or recursive makefiles. should be configurable.
   environment = os.environ.copy()
   environment['MAKEFLAGS'] = '-j%r' % multiprocessing.cpu_count()
   slapgrid = Popen([config['slapgrid_sr'], '-vc',
@@ -249,30 +261,30 @@ def runSoftwareWithLock(config):
   #Saves the current compile software for re-use
   config_SR_folder(config)
   return True
-  return False
+
 
 def config_SR_folder(config):
-  """Create a symbolik link for each folder in software folder. That allow
-    user to customize software release folder"""
+  """Create a symbolik link for each folder in software folder. That allows
+    the user to customize software release folder"""
   list = []
+  # XXX-Marco do not shadow 'list'
   config_name = 'slaprunner.config'
   for path in os.listdir(config['software_link']):
     cfg_path = os.path.join(config['software_link'], path, config_name)
     if os.path.exists(cfg_path):
-      cfg = open(cfg_path, 'r').read().split("#")
+      cfg = open(cfg_path).read().split("#")
       if len(cfg) != 2:
-        continue #there is a broken config file
+        continue  # there is a broken config file
       list.append(cfg[1])
   folder_list = os.listdir(config['software_root'])
-  if len(folder_list) < 1:
+  if not folder_list:
     return
-  curent_project = open(os.path.join(config['etc_dir'], ".project"),
-                        'r').read()
-  projects = curent_project.split("/")
-  name = projects[len(projects) - 2]
+  current_project = open(os.path.join(config['etc_dir'], ".project")).read()
+  projects = current_project.split('/')
+  name = projects[-2]
   for folder in folder_list:
     if folder in list:
-      continue #this folder is already registered
+      continue  # this folder is already registered
     else:
       if not os.path.exists(os.path.join(config['software_link'], name)):
         destination = os.path.join(config['software_link'], name)
@@ -283,9 +295,9 @@ def config_SR_folder(config):
       #create symlink
       os.symlink(source, destination)
       #write config file
-      cf = open(cfg, 'w')
-      cf.write(curent_project+"#"+folder)
-      cf.close()
+      with open(cfg, 'w') as cf:
+        cf.write(current_project + '#' + folder)
+
 
 def loadSoftwareRList(config):
   """Return list (of dict) of Software Release from symbolik SR folder"""
@@ -294,16 +306,18 @@ def loadSoftwareRList(config):
   for path in os.listdir(config['software_link']):
     cfg_path = os.path.join(config['software_link'], path, config_name)
     if os.path.exists(cfg_path):
-      cfg = open(cfg_path, 'r').read().split("#")
+      cfg = open(cfg_path).read().split("#")
       if len(cfg) != 2:
-        continue #there is a broken config file
+        continue  # there is a broken config file
       list.append(dict(md5=cfg[1], path=cfg[0], title=path))
   return list
 
+
 def isInstanceRunning(config=None):
   """
-    Return True if slapgrid-cp is still running and false if slapgrid if not
+    Return True if slapgrid-cp is still running and False otherwise
   """
+  # XXX-Marco what is 'config' for?
   return isRunning('slapgrid-cp')
 
 
@@ -312,8 +326,10 @@ def runInstanceWithLock(config):
     Use Slapgrid to deploy current Software Release and wait until
     deployment is done.
   """
+  if isInstanceRunning():
+    return False
+
   slapgrid_pid = os.path.join(config['run_dir'], 'slapgrid-cp.pid')
-  if not isInstanceRunning():
   startProxy(config)
   logfile = open(config['instance_log'], 'w')
   if not (updateProxy(config) and requestInstance(config)):
@@ -324,7 +340,7 @@ def runInstanceWithLock(config):
                    stdout=logfile, name='slapgrid-cp')
   slapgrid.wait()
   return True
-  return False
+
 
 def getProfilePath(projectDir, profile):
   """
@@ -342,6 +358,7 @@ def getProfilePath(projectDir, profile):
   projectFolder = open(os.path.join(projectDir, ".project")).read()
   return os.path.join(projectFolder, profile)
 
+
 def getSlapStatus(config):
   """Return all Slapos Partitions with associate information"""
   slap = slapos.slap.slap()
@@ -361,23 +378,26 @@ def getSlapStatus(config):
         partition_list.append((slappart_id, []))
   return partition_list
 
+
 def svcStopAll(config):
-  """Stop all Instance process on this computer"""
+  """Stop all Instance processes on this computer"""
   return Popen([config['supervisor'], config['configuration_file_path'],
                 'shutdown']).communicate()[0]
 
+
 def removeInstanceRoot(config):
-  """Clean instance directory and stop all its running process"""
+  """Clean instance directory and stop all its running processes"""
   if os.path.exists(config['instance_root']):
     svcStopAll(config)
     for root, dirs, _ in os.walk(config['instance_root']):
       for fname in dirs:
         fullPath = os.path.join(root, fname)
-        if not os.access(fullPath, os.W_OK) :
+        if not os.access(fullPath, os.W_OK):
           # Some directories may be read-only, preventing to remove files in it
           os.chmod(fullPath, 0744)
     shutil.rmtree(config['instance_root'])
 
+
 def getSvcStatus(config):
   """Return all Softwares Instances process Information"""
   result = Popen([config['supervisor'], config['configuration_file_path'],
@@ -386,22 +406,24 @@ def getSvcStatus(config):
   supervisord = []
   for item in result.split('\n'):
     if item.strip() != "":
-      if re.search(regex, item, re.IGNORECASE) == None:
+      if re.search(regex, item, re.IGNORECASE) is None:
         supervisord.append(re.split('[\s,]+', item))
   return supervisord
 
+
 def getSvcTailProcess(config, process):
-  """Get log for the specifie process
+  """Get log for the specified process
 
   Args:
     config: Slaprunner configuration
-    process: process name. this value is pass to supervisord.
+    process: process name. this value is passed to supervisord.
   Returns:
     a string that contains the log of the process.
   """
   return Popen([config['supervisor'], config['configuration_file_path'],
                 "tail", process]).communicate()[0]
 
+
 def svcStartStopProcess(config, process, action):
   """Send start or stop process command to supervisord
 
@@ -410,10 +432,17 @@ def svcStartStopProcess(config, process, action):
     process: process to start or stop.
     action: current state which is used to generate the new process state.
   """
-  cmd = {"RESTART":"restart", "STOPPED":"start", "RUNNING":"stop", "EXITED":"start", "STOP":"stop"}
+  cmd = {
+    'RESTART': 'restart',
+    'STOPPED': 'start',
+    'RUNNING': 'stop',
+    'EXITED': 'start',
+    'STOP': 'stop'
+  }
   return Popen([config['supervisor'], config['configuration_file_path'],
                 cmd[action], process]).communicate()[0]
 
+
 def getFolderContent(config, folder):
   """
   Read all file and folder into specified directory
@@ -423,7 +452,7 @@ def getFolderContent(config, folder):
     folder: the directory to read.
 
   Returns:
-    Html formated string or error message when fail.
+    Html formatted string or error message when fail.
   """
   r = ['<ul class="jqueryFileTree" style="display: none;">']
   try:
@@ -431,13 +460,14 @@ def getFolderContent(config, folder):
     r = ['<ul class="jqueryFileTree" style="display: none;">']
     d = urllib.unquote(folder)
     realdir = realpath(config, d)
-    if not realdir:
+    if realdir:
+      ldir = sorted(os.listdir(realdir), key=str.lower)
+    else:
       r.append('Could not load directory: Permission denied')
       ldir = []
-    else:
-      ldir = sorted(os.listdir(realdir), key=str.lower)
+
     for f in ldir:
-      if f.startswith('.'): #do not displays this file/folder
+      if f.startswith('.'):  # do not displays this file/folder
         continue
       ff = os.path.join(d, f)
       if os.path.isdir(os.path.join(realdir, f)):
@@ -451,6 +481,7 @@ def getFolderContent(config, folder):
   r.append('</ul>')
   return jsonify(result=''.join(r))
 
+
 def getFolder(config, folder):
   """
   Read list of folder for the specified directory
@@ -460,7 +491,7 @@ def getFolder(config, folder):
     folder: the directory to read.
 
   Returns:
-    Html formated string or error message when fail.
+    Html formatted string or error message when fail.
   """
   r = ['<ul class="jqueryFileTree" style="display: none;">']
   try:
@@ -474,7 +505,7 @@ def getFolder(config, folder):
     else:
       ldir = sorted(os.listdir(realdir), key=str.lower)
     for f in ldir:
-      if f.startswith('.'): #do not display this file/folder
+      if f.startswith('.'):  # do not display this file/folder
         continue
       ff = os.path.join(d, f)
       if os.path.isdir(os.path.join(realdir, f)):
@@ -485,6 +516,7 @@ def getFolder(config, folder):
   r.append('</ul>')
   return jsonify(result=''.join(r))
 
+
 def getProjectList(folder):
   """Return the list of projet (folder) into the workspace
 
@@ -500,6 +532,7 @@ def getProjectList(folder):
       project.append(elt)
   return project
 
+
 def configNewSR(config, projectpath):
   """Configure a Software Release as current Software Release
 
@@ -527,6 +560,7 @@ def configNewSR(config, projectpath):
   else:
     return False
 
+
 def newSoftware(folder, config, session):
   """
   Create a new Software Release folder with default profiles
@@ -562,14 +596,14 @@ def newSoftware(folder, config, session):
       session['title'] = getProjectTitle(config)
       code = 1
     else:
-      json = "Bad folder or Directory '" + folder + \
-        "' already exist, please enter a new name for your software"
+      json = "Bad folder or Directory '%s' already exist, please enter a new name for your software" % folder
   except Exception as e:
-    json = "Can not create your software, please try again! : " + str(e)
+    json = "Can not create your software, please try again! : %s " % e
     if os.path.exists(folderPath):
       shutil.rmtree(folderPath)
   return jsonify(code=code, result=json)
 
+
 def checkSoftwareFolder(path, config):
   """Check id `path` is a valid Software Release folder"""
   realdir = realpath(config, path)
@@ -577,6 +611,7 @@ def checkSoftwareFolder(path, config):
     return jsonify(result=path)
   return jsonify(result="")
 
+
 def getProjectTitle(config):
   """Generate the name of the current software Release (for slaprunner UI)"""
   conf = os.path.join(config['etc_dir'], ".project")
@@ -586,6 +621,7 @@ def getProjectTitle(config):
     return '%s (%s)' % (software, '/'.join(project[:-2]))
   return "No Profile"
 
+
 def getSoftwareReleaseName(config):
   """Get the name of the current Software Release"""
   sr_profile = os.path.join(config['etc_dir'], ".project")
@@ -595,6 +631,7 @@ def getSoftwareReleaseName(config):
     return software.replace(' ', '_')
   return "No_name"
 
+
 def removeSoftwareByName(config, md5, folderName):
   """Remove all content of the software release specified by md5
 
@@ -610,12 +647,13 @@ def removeSoftwareByName(config, md5, folderName):
     raise Exception("Cannot remove software Release: No such file or directory")
   if not os.path.exists(linkpath):
     raise Exception("Cannot remove software Release: No such file or directory %s" %
-                    ('software_root/'+folderName))
+                    ('software_root/' + folderName))
   svcStopAll(config)
   os.unlink(linkpath)
   shutil.rmtree(path)
   return loadSoftwareRList(config)
 
+
 def tail(f, lines=20):
   """
   Returns the last `lines` lines of file `f`. It is an implementation of tail -f n.
@@ -643,30 +681,32 @@ def tail(f, lines=20):
       block -= 1
   return '\n'.join(''.join(data).splitlines()[-lines:])
 
+
 def readFileFrom(f, lastPosition, limit=20000):
   """
   Returns the last lines of file `f`, from position lastPosition.
   and the last position
-  limit = max number of caracter to read
+  limit = max number of characters to read
   """
   BUFSIZ = 1024
   f.seek(0, 2)
+  # XXX-Marco do now shadow 'bytes'
   bytes = f.tell()
   block = -1
   data = ""
   length = bytes
-  truncated = False #True if a part of log data has been truncated
-  if (lastPosition <= 0 and length > limit) or (length-lastPosition > limit):
+  truncated = False  # True if a part of log data has been truncated
+  if (lastPosition <= 0 and length > limit) or (length - lastPosition > limit):
     lastPosition = length - limit
     truncated = True
   size = bytes - lastPosition
   while bytes > lastPosition:
-    if abs(block*BUFSIZ) <= size:
+    if abs(block * BUFSIZ) <= size:
       # Seek back one whole BUFSIZ
       f.seek(block * BUFSIZ, 2)
       data = f.read(BUFSIZ) + data
     else:
-      margin = abs(block*BUFSIZ) - size
+      margin = abs(block * BUFSIZ) - size
       if length < BUFSIZ:
         f.seek(0, 0)
       else:
@@ -676,7 +716,12 @@ def readFileFrom(f, lastPosition, limit=20000):
     bytes -= BUFSIZ
     block -= 1
   f.close()
-  return {"content":data, "position":length, "truncated":truncated}
+  return {
+    'content': data,
+    'position': length,
+    'truncated': truncated
+  }
+
 
 def isText(file):
   """Return True if the mimetype of file is Text"""
@@ -689,8 +734,10 @@ def isText(file):
   except:
     return False
 
+
 def md5sum(file):
   """Compute md5sum of `file` and return hexdigest value"""
+  # XXX-Marco: returning object or False boolean is an anti-pattern. better to return object or None
   if os.path.isdir(file):
     return False
   try:
@@ -705,6 +752,7 @@ def md5sum(file):
   except:
     return False
 
+
 def realpath(config, path, check_exist=True):
   """
   Get realpath of path or return False if user is not allowed to access to
@@ -712,10 +760,15 @@ def realpath(config, path, check_exist=True):
   """
   split_path = path.split('/')
   key = split_path[0]
-  allow_list = {'software_root':config['software_root'], 'instance_root':
-                config['instance_root'], 'workspace': config['workspace'],
-                'software_link':config['software_link']}
-  if allow_list.has_key(key):
+  allow_list = {
+    'software_root': config['software_root'],
+    'instance_root': config['instance_root'],
+    'workspace': config['workspace'],
+    'software_link': config['software_link']
+  }
+  if key not in allow_list:
+    return False
+
   del split_path[0]
   path = os.path.join(allow_list[key], *split_path)
   if check_exist:
@@ -725,7 +778,7 @@ def realpath(config, path, check_exist=True):
       return False
   else:
     return path
-  return False
+
 
 def readParameters(path):
   """Read Instance parameters stored into a local file.
@@ -734,7 +787,7 @@ def readParameters(path):
     path: path of the xml file that contains parameters
 
   Return:
-    a dictionnary of instance parameters."""
+    a dictionary of instance parameters."""
   if os.path.exists(path):
     try:
       xmldoc = minidom.parse(path)
@@ -743,10 +796,10 @@ def readParameters(path):
         sub_obj = {}
         for subnode in elt.childNodes:
           if subnode.nodeType != subnode.TEXT_NODE:
-            sub_obj[str(subnode.getAttribute('id'))] = subnode.childNodes[0].data #.decode('utf-8').decode('utf-8')
+            sub_obj[str(subnode.getAttribute('id'))] = subnode.childNodes[0].data  # .decode('utf-8').decode('utf-8')
             obj[str(elt.tagName)] = sub_obj
       return obj
     except Exception, e:
       return str(e)
   else:
-    return "No such file or directory: " + path
+    return "No such file or directory: %s" % path

slapos/runner/views.py

@@ -41,13 +41,17 @@ file_request = FileBrowser(app.config)
 import logging
 logger = logging.getLogger('werkzeug')
 
+
 def login_redirect(*args, **kwargs):
   return redirect(url_for('login'))
 
+
 #Access Control: Only static files and login pages are allowed to guest
 @app.before_request
 def before_request():
-  if not request.path.startswith('/static'):
+  if request.path.startswith('/static'):
+    return
+
   account = getSession(app.config)
   if account:
     user = AuthUser(username=account[0])
@@ -59,20 +63,24 @@ def before_request():
     if request.path != "/setAccount" and request.path != "/configAccount":
       return redirect(url_for('setAccount'))
 
+
 # general views
 @login_required()
 def home():
   return render_template('index.html')
 
+
 # general views
 @login_required()
 def browseWorkspace():
   return render_template('workspace.html')
 
+
 @app.route("/login")
 def login():
   return render_template('login.html')
 
+
 @app.route("/setAccount")
 def setAccount():
   account = getSession(app.config)
@@ -80,25 +88,29 @@ def setAccount():
     return render_template('account.html')
   return redirect(url_for('login'))
 
+
 @login_required()
 def myAccount():
   account = getSession(app.config)
   return render_template('account.html', username=account[0],
           email=account[2], name=account[3].decode('utf-8'))
 
+
 @app.route("/dologout")
 def dologout():
   _ = logout()
   return redirect(url_for('login'))
 
+
 @login_required()
 def configRepo():
-  public_key = open(app.config['public_key'], 'r').read()
+  public_key = open(app.config['public_key']).read()
   account = getSession(app.config)
   return render_template('cloneRepository.html', workDir='workspace',
             public_key=public_key, name=account[3].decode('utf-8'),
             email=account[2])
 
+
 @app.route("/doLogin", methods=['POST'])
 def doLogin():
   username = request.form['clogin']
@@ -106,7 +118,8 @@ def doLogin():
     # Authenticate and log in!
     if g.users[username].authenticate(request.form['cpwd']):
       return jsonify(code=1, result="")
-  return jsonify(code=0, result="Login or password is incorrect, please check it!")
+  return jsonify(code=0, result="Login or password is incorrect, please check it!"), 401
+
 
 # software views
 @login_required()
@@ -117,11 +130,13 @@ def editSoftwareProfile():
   return render_template('updateSoftwareProfile.html', workDir='workspace',
       profile=profile, projectList=getProjectList(app.config['workspace']))
 
+
 @login_required()
 def inspectSoftware():
   return render_template('runResult.html', softwareRoot='software_link/',
                          softwares=loadSoftwareRList(app.config))
 
+
 #remove content of compiled software release
 @login_required()
 def removeSoftware():
@@ -135,22 +150,25 @@ def removeSoftware():
     flash('Software removed')
   return redirect(url_for('inspectSoftware'))
 
+
 @login_required()
 def runSoftwareProfile():
   if runSoftwareWithLock(app.config):
-    return  jsonify(result = True)
+    return jsonify(result=True)
   else:
-    return  jsonify(result = False)
+    return jsonify(result=False)
+
 
 @login_required()
 def viewSoftwareLog():
   if os.path.exists(app.config['software_log']):
-    result = tail(open(app.config['software_log'], 'r'), lines=1500)
+    result = tail(open(app.config['software_log']), lines=1500)
   else:
     result = 'Not found yet'
   return render_template('viewLog.html', type='software',
       result=result.encode("utf-8"))
 
+
 # instance views
 @login_required()
 def editInstanceProfile():
@@ -160,50 +178,48 @@ def editInstanceProfile():
   return render_template('updateInstanceProfile.html', workDir='workspace',
       profile=profile, projectList=getProjectList(app.config['workspace']))
 
+
 # get status of all computer partitions and process state
 @login_required()
 def inspectInstance():
-  file_content = ''
-  result = ''
   if os.path.exists(app.config['instance_root']):
-    file_content = 'instance_root'
-    result = getSvcStatus(app.config)
-    if len(result) == 0:
-      result = []
+    file_path = 'instance_root'
+    supervisor = getSvcStatus(app.config)
+  else:
+    file_path = ''
+    supervisor = []
   return render_template('instanceInspect.html',
-      file_path=file_content, supervisor=result,
+                         file_path=file_path,
+                         supervisor=supervisor,
                          slap_status=getSlapStatus(app.config),
-      supervisore=result, partition_amount=app.config['partition_amount'])
+                         partition_amount=app.config['partition_amount'])
+
 
 #Reload instance process ans returns new value to ajax
 @login_required()
 def supervisordStatus():
   result = getSvcStatus(app.config)
-  if not (result):
+  if not result:
     return jsonify(code=0, result="")
+  # XXX-Marco -> template
   html = "<tr><th>Partition and Process name</th><th>Status</th><th>Process PID </th><th> UpTime</th><th></th></tr>"
   for item in result:
     html += "<tr>"
-    html += "<td  class='first'><b><a href='" \
-        + url_for('tailProcess', process=item[0]) + "'>" \
-        + item[0] + "</a></b></td>"
-    html += "<td align='center'><a href='" \
-        + url_for('startStopProccess', process=item[0], action=item[1]) \
-        + "'>" + item[1] + "</a></td>"
+    html += "<td  class='first'><b><a href='" + url_for('tailProcess', process=item[0]) + "'>" + item[0] + "</a></b></td>"
+    html += "<td align='center'><a href='" + url_for('startStopProccess', process=item[0], action=item[1]) + "'>" + item[1] + "</a></td>"
     html += "<td align='center'>" + item[3] + "</td><td>" + item[5] + "</td>"
-    html += "<td align='center'><a href='" \
-        + url_for('startStopProccess', process=item[0], action='RESTART') \
-        + "'>Restart</a></td>"
+    html += "<td align='center'><a href='" + url_for('startStopProccess', process=item[0], action='RESTART') + "'>Restart</a></td>"
     html += "</tr>"
   return jsonify(code=1, result=html)
 
+
 @login_required()
 def removeInstance():
   if isInstanceRunning(app.config):
     flash('Instantiation in progress, cannot remove')
   else:
     removeProxyDb(app.config)
-    svcStopAll(app.config) #Stop All instance process
+    svcStopAll(app.config)  # Stop All instance process
     removeInstanceRoot(app.config)
     param_path = os.path.join(app.config['etc_dir'], ".parameter.xml")
     if os.path.exists(param_path):
@@ -211,67 +227,83 @@ def removeInstance():
     flash('Instance removed')
   return redirect(url_for('inspectInstance'))
 
+
 @login_required()
 def runInstanceProfile():
   if not os.path.exists(app.config['instance_root']):
     os.mkdir(app.config['instance_root'])
   if runInstanceWithLock(app.config):
-    return  jsonify(result = True)
+    return jsonify(result=True)
   else:
-    return  jsonify(result = False)
+    return jsonify(result=False)
+
 
 @login_required()
 def viewInstanceLog():
   if os.path.exists(app.config['instance_log']):
-    result = open(app.config['instance_log'], 'r').read()
+    result = open(app.config['instance_log']).read()
   else:
     result = 'Not found yet'
   return render_template('viewLog.html', type='instance',
       result=result.encode("utf-8"))
 
+
 @login_required()
 def stopAllPartition():
   svcStopAll(app.config)
   return redirect(url_for('inspectInstance'))
 
+
 @login_required(login_redirect)
 def tailProcess(process):
   return render_template('processTail.html',
       process_log=getSvcTailProcess(app.config, process), process=process)
 
+
 @login_required(login_redirect)
 def startStopProccess(process, action):
   svcStartStopProcess(app.config, process, action)
   return redirect(url_for('inspectInstance'))
 
+
 @login_required(login_redirect)
 def openProject(method):
   return render_template('projectFolder.html', method=method,
                          workDir='workspace')
 
+
 @login_required()
 def cloneRepository():
   path = realpath(app.config, request.form['name'], False)
-  data = {"repo":request.form['repo'], "user":request.form['user'],
-          "email":request.form['email'], "path":path}
+  data = {
+    'repo': request.form['repo'],
+    'user': request.form['user'],
+    'email': request.form['email'],
+    'path': path
+  }
   return cloneRepo(data)
 
+
 @login_required()
 def readFolder():
   return getFolderContent(app.config, request.form['dir'])
 
+
 @login_required()
 def openFolder():
   return getFolder(app.config, request.form['dir'])
 
+
 @login_required()
 def createSoftware():
   return newSoftware(request.form['folder'], app.config, session)
 
+
 @login_required()
 def checkFolder():
   return checkSoftwareFolder(request.form['path'], app.config)
 
+
 @login_required()
 def setCurrentProject():
   if configNewSR(app.config, request.form['path']):
@@ -280,11 +312,13 @@ def setCurrentProject():
   else:
     return jsonify(code=0, result=("Can not setup this Software Release"))
 
+
 @login_required()
 def manageProject():
   return render_template('manageProject.html', workDir='workspace',
                          project=getProjectList(app.config['workspace']))
 
+
 @login_required()
 def getProjectStatus():
   path = realpath(app.config, request.form['project'])
@@ -293,6 +327,7 @@ def getProjectStatus():
   else:
     return jsonify(code=0, result="Can not read folder: Permission Denied")
 
+
 #view for current software release files
 @login_required()
 def editCurrentProject():
@@ -303,13 +338,13 @@ def editCurrentProject():
                            projectList=getProjectList(app.config['workspace']))
   return redirect(url_for('configRepo'))
 
+
 #create file or directory
 @login_required()
 def createFile():
   path = realpath(app.config, request.form['file'], False)
   if not path:
-    return jsonify(code=0, result="Error when creating your " + \
-                   request.form['type'] + ": Permission Denied")
+    return jsonify(code=0, result="Error when creating your %s: Permission Denied" % request.form['type'])
   try:
     if request.form['type'] == "file":
       open(path, 'w')
@@ -319,6 +354,7 @@ def createFile():
   except Exception as e:
     return jsonify(code=0, result=str(e))
 
+
 #remove file or directory
 @login_required()
 def removeFile():
@@ -331,6 +367,7 @@ def removeFile():
   except Exception as e:
     return jsonify(code=0, result=str(e))
 
+
 @login_required()
 def removeSoftwareDir():
   try:
@@ -340,6 +377,7 @@ def removeSoftwareDir():
   except Exception as e:
     return jsonify(code=0, result=str(e))
 
+
 #read file and return content to ajax
 @login_required()
 def getFileContent():
@@ -348,14 +386,15 @@ def getFileContent():
     if not isText(file_path):
       return jsonify(code=0,
             result="Can not open a binary file, please select a text file!")
-    if not request.form.has_key('truncate'):
-      return jsonify(code=1, result=open(file_path, 'r').read())
-    else:
-      content = tail(open(file_path, 'r'), int(request.form['truncate']))
+    if 'truncate' in request.form:
+      content = tail(open(file_path), int(request.form['truncate']))
       return jsonify(code=1, result=content)
+    else:
+      return jsonify(code=1, result=open(file_path).read())
   else:
     return jsonify(code=0, result="Error: No such file!")
 
+
 @login_required()
 def saveFileContent():
   file_path = realpath(app.config, request.form['file'])
@@ -365,6 +404,7 @@ def saveFileContent():
   else:
     return jsonify(code=0, result="Error: No such file!")
 
+
 @login_required()
 def changeBranch():
   path = realpath(app.config, request.form['project'])
@@ -373,6 +413,7 @@ def changeBranch():
   else:
     return jsonify(code=0, result="Can not read folder: Permission Denied")
 
+
 @login_required()
 def newBranch():
   path = realpath(app.config, request.form['project'])
@@ -384,6 +425,7 @@ def newBranch():
   else:
     return jsonify(code=0, result="Can not read folder: Permission Denied")
 
+
 @login_required(login_redirect)
 def getProjectDiff(project):
   path = os.path.join(app.config['workspace'], project)
@@ -398,6 +440,7 @@ def commitProjectFiles():
   else:
     return jsonify(code=0, result="Can not read folder: Permission Denied")
 
+
 @login_required()
 def pushProjectFiles():
   path = realpath(app.config, request.form['project'])
@@ -406,6 +449,7 @@ def pushProjectFiles():
   else:
     return jsonify(code=0, result="Can not read folder: Permission Denied")
 
+
 @login_required()
 def pullProjectFiles():
   path = realpath(app.config, request.form['project'])
@@ -414,6 +458,7 @@ def pullProjectFiles():
   else:
     return jsonify(code=0, result="Can not read folder: Permission Denied")
 
+
 @login_required()
 def checkFileType():
   path = realpath(app.config, request.form['path'])
@@ -425,6 +470,7 @@ def checkFileType():
     return jsonify(code=0,
                    result="Can not open a binary file, please select a text file!")
 
+
 @login_required()
 def getmd5sum():
   realfile = realpath(app.config, request.form['file'])
@@ -436,26 +482,32 @@ def getmd5sum():
   else:
     return jsonify(code=0, result="Can not get md5sum for this file!")
 
+
 #return information about state of slapgrid process
 @login_required()
 def slapgridResult():
   software_state = isSoftwareRunning(app.config)
   instance_state = isInstanceRunning(app.config)
-  log_result = {"content":"", "position":0, "truncated":False}
-  if request.form['log'] == "software"  or\
-     request.form['log'] == "instance":
+  log_result = {
+    'content': '',
+    'position': 0,
+    'truncated': False
+  }
+  if request.form['log'] in ['software', 'instance']:
     log_file = request.form['log'] + "_log"
     if os.path.exists(app.config[log_file]):
-      log_result = readFileFrom(open(app.config[log_file], 'r'),
+      log_result = readFileFrom(open(app.config[log_file]),
                                 int(request.form['position']))
   return jsonify(software=software_state, instance=instance_state,
                  result=(instance_state or software_state), content=log_result)
 
+
 @login_required()
 def stopSlapgrid():
   result = killRunningProcess(request.form['type'])
   return jsonify(result=result)
 
+
 @login_required()
 def getPath():
   files = request.form['file'].split('#')
@@ -474,6 +526,7 @@ def getPath():
   else:
     return jsonify(code=1, result=realfile)
 
+
 @login_required()
 def saveParameterXml():
   """
@@ -485,9 +538,8 @@ def saveParameterXml():
   content = request.form['parameter'].encode("utf-8")
   param_path = os.path.join(app.config['etc_dir'], ".parameter.xml")
   try:
-    f = open(param_path, 'w')
+    with open(param_path, 'w') as f:
       f.write(content)
-    f.close()
     result = readParameters(param_path)
   except Exception as e:
     result = str(e)
@@ -502,26 +554,27 @@ def saveParameterXml():
     except Exception as e:
       return jsonify(
         code=0,
-        result="An error occurred while applying your settings!<br/>" + str(e))
+        result="An error occurred while applying your settings!<br/>%s" % e)
     return jsonify(code=1, result="")
 
+
 @login_required()
 def getSoftwareType():
   software_type_path = os.path.join(app.config['etc_dir'], ".software_type.xml")
   if os.path.exists(software_type_path):
-    return jsonify(code=1, result=open(software_type_path, 'r').read())
+    return jsonify(code=1, result=open(software_type_path).read())
   return jsonify(code=1, result="default")
 
+
 #read instance parameters into the local xml file and return a dict
 @login_required()
 def getParameterXml(request):
   param_path = os.path.join(app.config['etc_dir'], ".parameter.xml")
   if not os.path.exists(param_path):
-    default = '<?xml version="1.0" encoding="utf-8"?>\n'
-    default += '<instance>\n</instance>'
+    default = '<?xml version="1.0" encoding="utf-8"?>\n<instance>\n</instance>'
     return jsonify(code=1, result=default)
   if request == "xml":
-    parameters = open(param_path, 'r').read()
+    parameters = open(param_path).read()
   else:
     parameters = readParameters(param_path)
   if type(parameters) == type('') and request != "xml":
@@ -529,29 +582,36 @@ def getParameterXml(request):
   else:
     return jsonify(code=1, result=parameters)
 
+
 #update user account data
 @login_required()
 def updateAccount():
-  account = []
-  account.append(request.form['username'].strip())
-  account.append(request.form['password'].strip())
-  account.append(request.form['email'].strip())
-  account.append(request.form['name'].strip())
   code = request.form['rcode'].strip()
   recovery_code = open(os.path.join(app.config['etc_dir'], ".rcode"), "r").read()
   if code != recovery_code:
     return jsonify(code=0, result="Your password recovery code is not valid!")
+
+  account = [
+    request.form['username'].strip(),
+    request.form['password'].strip(),
+    request.form['email'].strip(),
+    request.form['name'].strip()
+  ]
   result = saveSession(app.config, account)
   if type(result) == type(""):
     return jsonify(code=0, result=result)
   else:
     return jsonify(code=1, result="")
 
+
 #update user account data
 @app.route("/configAccount", methods=['POST'])
 def configAccount():
   last_account = getSession(app.config)
-  if not last_account:
+  if last_account:
+    return jsonify(code=0,
+                   result="Unable to respond to your request, permission denied.")
+
   account = []
   account.append(request.form['username'].strip())
   account.append(request.form['password'].strip())
@@ -567,8 +627,7 @@ def configAccount():
     return jsonify(code=0, result=result)
   else:
     return jsonify(code=1, result="")
-  return jsonify(code=0,
-                 result="Unable to respond to your request, permission denied.")
+
 
 #Global File Manager
 @login_required()
@@ -584,6 +643,7 @@ def fileBrowser():
       opt = int(request.form['opt'])
   else:
     opt = int(request.args.get('opt'))
+
   try:
     if opt == 1:
       #list files and directories
@@ -634,6 +694,7 @@ def fileBrowser():
     return str(e)
   return result
 
+
 @login_required()
 def editFile():
   return render_template('editFile.html', workDir='workspace',
@@ -641,6 +702,7 @@ def editFile():
     projectList=getProjectList(app.config['workspace']),
     filename=urllib.unquote(request.args.get('filename', '')))
 
+
 #Setup List of URLs
 app.add_url_rule('/', 'home', home)
 app.add_url_rule('/browseWorkspace', 'browseWorkspace', browseWorkspace)