Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
language: python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wh should add unit test in python 3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already done :-), check below.

python:
- "2.7"
#- "3.4"
- "3.4"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come on, even Ubuntu is on 3.5 by now!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I added 3.5.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to ask for more than we need.
On top of that, this is a cluster tool and more often than not clusters have old version of everything.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smartdispatch can also be used on our own computer (e.g. use the useful expansion of folded argument to generate a commands file :-P) and I have a virtual env with Python 3.5.

Also, testing on Python 3.5 came for free with Travis, why not use take advantage of it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! You test it on both, nevermind I thought you removed 3.4. My bad

- "3.5"

install:
- pip install coveralls
Expand Down
20 changes: 13 additions & 7 deletions scripts/smart-dispatch
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env python2
#!/usr/bin/env python
# -*- coding: utf-8 -*-
from __future__ import print_function

import os
import sys
Expand Down Expand Up @@ -101,7 +102,7 @@ def main():

# Generating all the worker commands
worker_script = pjoin(os.path.dirname(smartdispatch.__file__), 'workers', 'base_worker.py')
COMMAND_STRING = 'cd "{cwd}"; python2 {worker_script} "{commands_file}" "{log_folder}" '\
COMMAND_STRING = 'cd "{cwd}"; python {worker_script} "{commands_file}" "{log_folder}" '\
'1>> "{log_folder}/worker/$PBS_JOBID\"\"_worker_{{ID}}.o" '\
'2>> "{log_folder}/worker/$PBS_JOBID\"\"_worker_{{ID}}.e" '
COMMAND_STRING = COMMAND_STRING.format(cwd=os.getcwd(), worker_script=worker_script, commands_file=command_manager._commands_filename, log_folder=path_job_logs)
Expand All @@ -119,19 +120,23 @@ def main():
pbs_filenames = job_generator.write_pbs_files(path_job_commands)

# Launch the jobs
print "## {nb_commands} command(s) will be executed in {nb_jobs} job(s) ##".format(nb_commands=nb_commands, nb_jobs=len(pbs_filenames))
print "Batch UID:\n{batch_uid}".format(batch_uid=jobname)
print("## {nb_commands} command(s) will be executed in {nb_jobs} job(s) ##".format(nb_commands=nb_commands, nb_jobs=len(pbs_filenames)))
print("Batch UID:\n{batch_uid}".format(batch_uid=jobname))
if not args.doNotLaunch:
jobs_id = []
for pbs_filename in pbs_filenames:
qsub_output = check_output('{launcher} {pbs_filename}'.format(launcher=LAUNCHER if args.launcher is None else args.launcher, pbs_filename=pbs_filename), shell=True)

if isinstance(qsub_output, bytes):
qsub_output = qsub_output.decode("utf-8")

jobs_id += [qsub_output.strip()]

with open_with_lock(pjoin(path_job, "jobs_id.txt"), 'a') as jobs_id_file:
jobs_id_file.writelines(t.strftime("## %Y-%m-%d %H:%M:%S ##\n"))
jobs_id_file.writelines("\n".join(jobs_id) + "\n")
print "\nJobs id:\n{jobs_id}".format(jobs_id=" ".join(jobs_id))
print "\nLogs, command, and jobs id related to this batch will be in:\n {smartdispatch_folder}".format(smartdispatch_folder=path_job)
print("\nJobs id:\n{jobs_id}".format(jobs_id=" ".join(jobs_id)))
print("\nLogs, command, and jobs id related to this batch will be in:\n {smartdispatch_folder}".format(smartdispatch_folder=path_job))


def parse_arguments():
Expand All @@ -146,7 +151,8 @@ def parse_arguments():
parser.add_argument('-c', '--coresPerCommand', type=int, required=False, help='How many cores a command needs.', default=1)
parser.add_argument('-g', '--gpusPerCommand', type=int, required=False, help='How many gpus a command needs.', default=1)
# parser.add_argument('-m', '--memPerCommand', type=float, required=False, help='How much memory a command needs (in Gb).')
parser.add_argument('-f', '--commandsFile', type=file, required=False, help='File containing commands to launch. Each command must be on a seperate line. (Replaces commandAndOptions)')
parser.add_argument('-f', '--commandsFile', type=argparse.FileType('r'), required=False,
help='File containing commands to launch. Each command must be on a seperate line. (Replaces commandAndOptions)')

parser.add_argument('-l', '--modules', type=str, required=False, help='List of additional modules to load.', nargs='+')
parser.add_argument('-x', '--doNotLaunch', action='store_true', help='Creates the QSUB files without launching them.')
Expand Down
2 changes: 1 addition & 1 deletion smartdispatch/argument_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def unfold(self, match):
start = int(groups[0])
end = int(groups[1])
step = 1 if groups[2] is None else int(groups[2])
return map(str, range(start, end, step))
return [str(i) for i in range(start, end, step)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were we really using map everywhere for no reasons ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no. In Python 3, map returns a generator instead of a list. What we want here is a list, so we are better off using list comprehension instead.



argument_templates = build_argument_templates_dictionnary()
2 changes: 1 addition & 1 deletion smartdispatch/smartdispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def unfold_command(command):
pos = match.end()

arguments.append([text[pos:]]) # Add remaining unfolded arguments
arguments = [map(utils.decode_escaped_characters, argvalues) for argvalues in arguments]
arguments = [[utils.decode_escaped_characters(v) for v in argvalues] for argvalues in arguments]
return ["".join(argvalues) for argvalues in itertools.product(*arguments)]


Expand Down
6 changes: 3 additions & 3 deletions smartdispatch/tests/test_filelock.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ def _test_open_with_lock(lock_func):
time.sleep(1)

stdout, stderr = process.communicate()
assert_equal(stdout, "")
assert_true("Traceback" not in stderr, msg="Unexpected error: '{}'".format(stderr))
assert_true("write-lock" in stderr, msg="Forcing a race condition, try increasing sleeping time above.")
assert_equal(stdout, b"")
assert_true("Traceback" not in stderr.decode(), msg="Unexpected error: '{}'".format(stderr.decode()))
assert_true("write-lock" in stderr.decode(), msg="Forcing a race condition, try increasing sleeping time above.")

shutil.rmtree(temp_dir) # Cleaning up.

Expand Down
6 changes: 3 additions & 3 deletions smartdispatch/tests/test_smartdispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import shutil
import time as t
from os.path import join as pjoin
from StringIO import StringIO
from io import StringIO, open

import tempfile
from nose.tools import assert_true, assert_equal
Expand Down Expand Up @@ -43,11 +43,11 @@ def test_get_commands_from_file():
commands = ["command1 arg1 arg2",
"command2",
"command3 arg1 arg2 arg3 arg4"]
fileobj = StringIO("\n".join(commands))
fileobj = StringIO(u"\n".join(commands))
assert_array_equal(smartdispatch.get_commands_from_file(fileobj), commands)

# Test stripping last line if empty
fileobj = StringIO("\n".join(commands) + "\n")
fileobj = StringIO(u"\n".join(commands) + u"\n")
assert_array_equal(smartdispatch.get_commands_from_file(fileobj), commands)


Expand Down
25 changes: 19 additions & 6 deletions smartdispatch/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from __future__ import print_function

import re
import binascii
import hashlib
import unicodedata
import json
Expand All @@ -15,7 +18,7 @@ def print_boxed(string):
out = u"\u250c" + box_line + u"\u2510\n"
out += '\n'.join([u"\u2502 {} \u2502".format(line.ljust(max_len)) for line in splitted_string])
out += u"\n\u2514" + box_line + u"\u2518"
print out
print(out)


def yes_no_prompt(query, default=None):
Expand All @@ -35,13 +38,13 @@ def yes_no_prompt(query, default=None):

def chunks(sequence, n):
""" Yield successive n-sized chunks from sequence. """
for i in xrange(0, len(sequence), n):
for i in range(0, len(sequence), n):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably use the six package for translation from python 2 to 3.
Especially in this case because in python2 the range will be super slow. On the other hand we probably don't need it to be that fast.

Copy link
Member Author

@MarcCote MarcCote Apr 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can avoid using six I think it is for the best. I would say we don't really care about speed here. Also, I don't think range in Python 2 is that slow.

yield sequence[i:i + n]


def generate_uid_from_string(value):
""" Create unique identifier from a string. """
return hashlib.sha256(value).hexdigest()
return hashlib.sha256(value.encode()).hexdigest()


def slugify(value):
Expand All @@ -54,15 +57,23 @@ def slugify(value):
---------
https://github.com/django/django/blob/1.7c3/django/utils/text.py#L436
"""
value = unicodedata.normalize('NFKD', unicode(value, "UTF-8")).encode('ascii', 'ignore').decode('ascii')
# Convert `value` to Unicode so we can slugify it using the unicodedata module.
try:
value = unicode(value, "UTF-8")
except NameError:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably a cleaner way to manage this. Maybe with six ?

Copy link
Member Author

@MarcCote MarcCote Apr 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

six module doesn't seem to have a unicode function, maybe it has another name.

pass # In Python 3, all strings are already stored as Unicode.

# Replace all compatibility characters with their equivalents.
value = unicodedata.normalize('NFKD', value).encode('ascii', 'ignore').decode('ascii')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like a hack. Maybe a comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I'll add one. I'm following the "It's easier to ask forgiveness than permission" principle here, it is not really a hack.

value = re.sub('[^\w\s-]', '', value).strip().lower()
return str(re.sub('[-\s]+', '_', value))


def encode_escaped_characters(text, escaping_character="\\"):
""" Escape the escaped character using its hex representation """
def hexify(match):
return "\\x{0}".format(match.group()[-1].encode("hex"))
# Reference: http://stackoverflow.com/questions/18298251/python-hex-values-to-convert-to-a-string-integer
return "\\x" + binascii.hexlify(match.group()[-1].encode()).decode()

return re.sub(r"\\.", hexify, text)

Expand All @@ -73,7 +84,7 @@ def decode_escaped_characters(text):
return ''

def unhexify(match):
return match.group()[2:].decode("hex")
return binascii.unhexlify(match.group()[2:]).decode()

return re.sub(r"\\x..", unhexify, text)

Expand All @@ -92,6 +103,8 @@ def detect_cluster():
# Get server status
try:
output = Popen(["qstat", "-B"], stdout=PIPE).communicate()[0]
if isinstance(output, bytes):
output = output.decode("utf-8")
except OSError:
# If qstat is not available we assume that the cluster is unknown.
return None
Expand Down
2 changes: 1 addition & 1 deletion smartdispatch/workers/base_worker.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python2
#!/usr/bin/env python
# -*- coding: utf-8 -*-

import os
Expand Down
12 changes: 6 additions & 6 deletions smartdispatch/workers/tests/test_base_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ def setUp(self):
self.command_manager = CommandManager(os.path.join(self._commands_dir, "commands.txt"))
self.command_manager.set_commands_to_run(self.commands)

self.commands_uid = map(utils.generate_uid_from_string, self.commands)
self.commands_uid = [utils.generate_uid_from_string(c) for c in self.commands]

def tearDown(self):
shutil.rmtree(self._commands_dir)
shutil.rmtree(self.logs_dir)

def test_main(self):
command = ['python2', self.base_worker_script, self.command_manager._commands_filename, self.logs_dir]
command = ['python', self.base_worker_script, self.command_manager._commands_filename, self.logs_dir]
assert_equal(call(command), 0)
# Simulate a resume, i.e. re-run the command, the output/error should be concatenated.
self.command_manager.set_commands_to_run(self.commands)
Expand Down Expand Up @@ -106,14 +106,14 @@ def test_main(self):
assert_equal("", logfile.read())

def test_lock(self):
command = ['python2', self.base_worker_script, self.command_manager._commands_filename, self.logs_dir]
command = ['python', self.base_worker_script, self.command_manager._commands_filename, self.logs_dir]

# Lock the commands file before running 'base_worker.py'
with open_with_lock(self.command_manager._commands_filename, 'r+'):
process = Popen(command, stdout=PIPE, stderr=PIPE)
time.sleep(1)

stdout, stderr = process.communicate()
assert_equal(stdout, "")
assert_true("write-lock" in stderr, msg="Forcing a race condition, try increasing sleeping time above.")
assert_true("Traceback" not in stderr) # Check that there are no errors.
assert_equal(stdout, b"")
assert_true("write-lock" in stderr.decode(), msg="Forcing a race condition, try increasing sleeping time above.")
assert_true("Traceback" not in stderr.decode()) # Check that there are no errors.