Commit 1dc1e5f2 authored by Nicola Soranzo's avatar Nicola Soranzo
Browse files

Python3: unicodify the output of ``subprocess.check_output()`` 

By default it's a bytestring.

Also:
- don't use `shell=True`
- add a `splitlines()` forgotten in commit 670897ec
- raise exception instead of returning `error_info` in `docker_to_singularity()`
- use a random temp dir in `singularity_container_test()`
parent e0b5097b

lib/galaxy/tool_util/deps/mulled/mulled_build.py

+2 −2
Original line number Diff line number Diff line
@@ -128,8 +128,8 @@ def get_affected_packages(args): 
    recipes_dir = args.recipes_dir

    hours = args.diff_hours

    cmd = ['git', 'log', '--diff-filter=ACMRTUXB', '--name-only', '--pretty=""', '--since="%s hours ago"' % hours]

    changed_files = subprocess.check_output(cmd, cwd=recipes_dir).strip().split('\n')

    pkg_list = set([x for x in changed_files if x.startswith('recipes/') and x.endswith('meta.yaml')])

    changed_files = unicodify(subprocess.check_output(cmd, cwd=recipes_dir)).splitlines()

    pkg_list = {x for x in changed_files if x.startswith('recipes/') and x.endswith('meta.yaml')}

    for pkg in pkg_list:

        if pkg and os.path.exists(os.path.join(recipes_dir, pkg)):

            yield (get_pkg_name(args, pkg), get_tests(args, pkg))

lib/galaxy/tool_util/deps/mulled/mulled_update_singularity_containers.py

+42 −46
Original line number Diff line number Diff line 
#!/usr/bin/env python



import argparse

import os

import subprocess

import tempfile

from glob import glob

from subprocess import check_output



from galaxy.util import unicodify

from .get_tests import hashed_test_search, main_test_search





def get_list_from_file(filename):

    r"""

    """

    Returns a list of containers stored in a file (one on each line)

    """

    return [n for n in open(filename).read().split('\n') if n != '']  # if blank lines are in the file empty strings must be removed

    with open(filename) as fh:

        return [_ for _ in fh.read().splitlines() if _]  # if blank lines are in the file empty strings must be removed





def docker_to_singularity(container, installation, filepath, no_sudo=False):

    """

    Convert docker to singularity container.

    """



    cmd = [installation, 'build', '/'.join((filepath, container)), 'docker://quay.io/biocontainers/' + container]

    try:

        if no_sudo:

            check_output("%s build %s/%s docker://quay.io/biocontainers/%s" % (installation, filepath, container, container), stderr=subprocess.STDOUT, shell=True)

            check_output(cmd, stderr=subprocess.STDOUT)

        else:

            check_output("sudo %s build %s/%s docker://quay.io/biocontainers/%s && sudo rm -rf /root/.singularity/docker/" % (installation, filepath, container, container), stderr=subprocess.STDOUT, shell=True)

            check_output(cmd.insert(0, 'sudo'), stderr=subprocess.STDOUT)

            check_output(['sudo', 'rm', '-rf', '/root/.singularity/docker/'], stderr=subprocess.STDOUT)

    except subprocess.CalledProcessError as e:

        error_info = {'code': e.returncode, 'cmd': e.cmd, 'out': e.output}

        return error_info

    else:

        return None

        raise Exception("Docker to Singularity conversion failed.\nOutput was:\n%" % unicodify(e.output))





def singularity_container_test(tests, installation, filepath):
@@ -40,40 +40,37 @@ def singularity_container_test(tests, installation, filepath): 
    test_results = {'passed': [], 'failed': [], 'notest': []}



    # create a 'sanitised home' directory in which the containers may be mounted - see http://singularity.lbl.gov/faq#solution-1-specify-the-home-to-mount

    os.mkdir("/tmp/sing_home")



    with tempfile.TemporaryDirectory() as tmpdirname:

        for container, test in tests.items():

            if 'commands' not in test and 'imports' not in test:

                test_results['notest'].append(container)



            else:

                exec_command = [installation, 'exec', '-H', tmpdirname, '/'.join((filepath, container))]

                test_passed = True

                errors = []

                if test.get('commands', False):

                for command in test['commands']:

                    command = command.replace('$PREFIX', '/usr/local/')

                    command = command.replace('${PREFIX}', '/usr/local/')

                    command = command.replace('$R ', 'Rscript ')

                    for test_command in test['commands']:

                        test_command = test_command.replace('$PREFIX', '/usr/local/')

                        test_command = test_command.replace('${PREFIX}', '/usr/local/')

                        test_command = test_command.replace('$R ', 'Rscript ')



                        try:

                        check_output("%s exec -H /tmp/sing_home %s/%s bash -c \"%s\"" % (

                            installation, filepath, container, command), stderr=subprocess.STDOUT, shell=True)

                            check_output(exec_command.extend(['bash', '-c', test_command]), stderr=subprocess.STDOUT)

                        except subprocess.CalledProcessError:

                            try:

                            check_output("%s exec -H /tmp/sing_home %s/%s %s" % (

                                installation, filepath, container, command), stderr=subprocess.STDOUT, shell=True)

                                check_output(exec_command.append(test_command), stderr=subprocess.STDOUT)

                            except subprocess.CalledProcessError as e:

                                errors.append(

                                {'command': command, 'output': e.output})

                                    {'command': test_command, 'output': unicodify(e.output)})

                                test_passed = False



                if test.get('imports', False):

                    for imp in test['imports']:

                        try:

                        check_output("%s exec -H /tmp/sing_home %s/%s %s 'import %s'" % (installation, filepath,

                                                                                         container, test['import_lang'], imp), stderr=subprocess.STDOUT, shell=True)

                            check_output(exec_command.extend([test['import_lang'], 'import ' + imp]), stderr=subprocess.STDOUT)

                        except subprocess.CalledProcessError as e:

                        errors.append({'import': imp, 'output': e.output})

                            errors.append({'import': imp, 'output': unicodify(e.output)})

                            test_passed = False



                if test_passed:
@@ -81,7 +78,6 @@ def singularity_container_test(tests, installation, filepath): 
                else:

                    test['errors'] = errors

                    test_results['failed'].append(test)

    os.rmdir("/tmp/sing_home")

    return test_results

lib/galaxy/webapps/reports/controllers/system.py

+3 −2
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ from sqlalchemy import and_, desc, false, null, true 
from sqlalchemy.orm import eagerload



from galaxy import model, util

from galaxy.util import unicodify

from galaxy.webapps.base.controller import BaseUIController, web



log = logging.getLogger(__name__)
@@ -151,9 +152,9 @@ class System(BaseUIController): 
    def get_disk_usage(self, file_path):

        is_sym_link = os.path.islink(file_path)

        file_system = disk_size = disk_used = disk_avail = disk_cap_pct = mount = None

        df_output = subprocess.check_output(['df', '-h', file_path])

        df_output = unicodify(subprocess.check_output(['df', '-h', file_path]))



        for df_line in df_output:

        for df_line in df_output.splitlines():

            df_line = df_line.strip()

            if df_line:

                df_line = df_line.lower()

lib/tool_shed/util/hg_util.py

+1 −1
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ def add_changeset(repo_path, path_to_filename_in_archive): 
    except Exception as e:

        error_message = "Error adding '%s' to repository: %s" % (path_to_filename_in_archive, unicodify(e))

        if isinstance(e, subprocess.CalledProcessError):

            error_message += "\nOutput was:\n%s" % e.output

            error_message += "\nOutput was:\n%s" % unicodify(e.output)

        raise Exception(error_message)

test/integration/test_kubernetes_runner.py

+6 −3
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ import time 


import pytest



from galaxy.util import unicodify

from galaxy_test.base.populators import skip_without_tool

from galaxy_test.driver import integration_util

from .test_containerized_jobs import MulledJobTestCases
@@ -235,7 +236,8 @@ class BaseKubernetesIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase, M 
                time.sleep(1)

                max_tries -= 1



            status = json.loads(subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json']))

            output = unicodify(subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json']))

            status = json.loads(output)

            assert status['status']['active'] == 1



            delete_response = self.dataset_populator.cancel_job(job_dict["id"])
@@ -246,7 +248,7 @@ class BaseKubernetesIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase, M 
            # The default job config removes jobs, didn't find a better way to check that the job doesn't exist anymore

            with pytest.raises(subprocess.CalledProcessError) as excinfo:

                subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json'], stderr=subprocess.STDOUT)

            assert "not found" in excinfo.value.output.decode()

            assert "not found" in unicodify(excinfo.value.output)



    @skip_without_tool('job_properties')

    def test_exit_code_127(self):
@@ -297,7 +299,8 @@ class BaseKubernetesIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase, M 
        job_dict = running_response["jobs"][0]

        job = self.galaxy_interactor.get("jobs/%s" % job_dict['id'], admin=True).json()

        external_id = job['external_id']

        status = json.loads(subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json']))

        output = unicodify(subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json']))

        status = json.loads(output)

        assert 'active' not in status['status']



    @skip_without_tool('create_2')