If spooldir defaults to a path in $HOME, there is no need for `_i…

…nit_spooldir()`.
gc3pie · Aug 11, 2018 · 3a15767 · 3a15767
1 parent ae0e1b7
commit 3a15767
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 106 deletions.
diff --git a/gc3libs/__init__.py b/gc3libs/__init__.py
@@ -60,11 +60,11 @@
 
 
 class Default(object):
-
     """
     A namespace for all constants and default values used in the
     GC3Libs package.
     """
+
     RCDIR = os.path.join(os.path.expandvars('$HOME'), ".gc3")
     CONFIG_FILE_LOCATIONS = [
         # system-wide config file
@@ -104,8 +104,13 @@ class Default(object):
 
     # time to cache lshosts/bjobs information for
     LSF_CACHE_TIME = 30
+
+    # root path for the working directory of jobs;
+    # on batch systems, this should be visible from both
+    # the frontend and the compute nodes
     SPOOLDIR = "$HOME/.gc3pie_jobs"
 
+
 from gc3libs.events import TaskStateChange
 import gc3libs.exceptions
 from gc3libs.persistence import Persistable

diff --git a/gc3libs/backends/batch.py b/gc3libs/backends/batch.py
@@ -33,7 +33,7 @@
 import uuid
 
 import gc3libs
-from gc3libs import log, Run
+from gc3libs import Default, log, Run
 from gc3libs.backends import LRMS
 from gc3libs.utils import same_docstring_as, sh_quote_safe
 import gc3libs.backends.transport
@@ -125,7 +125,7 @@ def __init__(self, name,
                  ssh_timeout=None,
                  large_file_threshold=None,
                  large_file_chunk_size=None,
-                 spooldir = None,
+                 spooldir=Default.SPOOLDIR,
                  **extra_args):
 
         # init base class
@@ -134,15 +134,9 @@ def __init__(self, name,
             architecture, max_cores, max_cores_per_job,
             max_memory_per_core, max_walltime, auth, **extra_args)
 
-
-        # default is to use $TMPDIR or '/var/tmp' (see
-        # `tempfile.mkftemp`), but we delay the determination of the
-        # correct dir to the submit_job, so that we don't have to use
-        # `transport` right now.
-        self.spooldir = spooldir
-
         # backend-specific setup
         self.frontend = frontend
+        self.spooldir = spooldir
         if transport == 'local':
             self.transport = gc3libs.backends.transport.LocalTransport()
             self._username = getuser()
@@ -166,43 +160,6 @@ def __init__(self, name,
         self.accounting_delay = accounting_delay
 
 
-    @property
-    def spooldir(self):
-        """
-        Root folder for all working directories of GC3Pie tasks.
-
-        When this backend executes a task, it first creates a temporary
-        subdirectory of this folder, then launches commands in there.
-
-        If not explicitly set (e.g. at construction time), the "spool
-        directory" will be given a default value according to the logic of
-        :meth:`_init_spooldir`:
-
-        * If the remote environment variable ``TMPDIR`` is set and points to an
-          existing directory, that value is used;
-        * otherwise, the hard-coded default ``/var/tmp`` is used instead.
-        """
-        if not self._spooldir:
-            self._init_spooldir()
-        return self._spooldir
-
-    @spooldir.setter
-    def spooldir(self, value):
-        self._spooldir = value
-
-    def _init_spooldir(self):
-        """Set `self.spooldir` to a sensible value."""
-        self.transport.connect()
-        rc, stdout, stderr = self.transport.execute_command(
-            'cd "{0}" && pwd'.format(gc3libs.Default.SPOOLDIR))
-        if (rc != 0 or stdout.strip() == '' or stdout[0] != '/'):
-            raise gc3libs.exceptions.SpoolDirError("Unable to use {0} for `spooldir` "
-                                                   "on resource {0}. ".format(gc3libs.Default.SPOOLDIR,
-                                                                              self.name
-                                                                              ))
-        else:
-            self.spooldir = stdout.strip()
-
     def get_jobid_from_submit_output(self, output, regexp):
         """Parse the output of the submission command. Regexp is
         provided by the caller. """
@@ -448,17 +405,20 @@ def submit_job(self, app):
         # Create the remote directory.
         try:
             self.transport.connect()
-            cmd = "mkdir -p {0};" \
-                " mktemp -d {0}/lrms_job.XXXXXXXXXX".format(self.spooldir)
-            log.info("Creating remote temporary folder: command '%s' " % cmd)
+            cmd = (
+                "mkdir -p {0};"
+                " mktemp -d {0}/lrms_job.XXXXXXXXXX"
+                .format(self.spooldir))
+            log.info("Creating temporary job working directory")
             exit_code, stdout, stderr = self.transport.execute_command(cmd)
             if exit_code == 0:
                 ssh_remote_folder = stdout.split('\n')[0]
             else:
-                raise gc3libs.exceptions.LRMSError(
-                    "Failed executing command '%s' on resource '%s';"
-                    " exit code: %d, stderr: '%s'."
-                    % (cmd, self.name, exit_code, stderr))
+                raise gc3libs.exceptions.SpoolDirError(
+                    "Cannot create temporary job working directory"
+                    " on resource '%s'; command '%s' exited"
+                    " with code: %d and stderr: '%s'."
+                    % (self.name, cmd, exit_code, stderr))
         except gc3libs.exceptions.TransportError:
             raise
         except:

diff --git a/gc3libs/backends/shellcmd.py b/gc3libs/backends/shellcmd.py
@@ -45,7 +45,7 @@
 import gc3libs
 import gc3libs.exceptions
 import gc3libs.backends.transport
-from gc3libs import log, Run
+from gc3libs import Default, log, Run
 from gc3libs.utils import same_docstring_as, Struct, sh_quote_safe, sh_quote_unsafe
 from gc3libs.backends import LRMS
 from gc3libs.quantity import Duration, Memory, MB
@@ -570,7 +570,7 @@ def __init__(self, name,
                  frontend='localhost', transport='local',
                  time_cmd=None,
                  override='False',
-                 spooldir=None,
+                 spooldir=Default.SPOOLDIR,
                  resourcedir=None,
                  # SSH-related options; ignored if `transport` is 'local'
                  ssh_config=None,
@@ -591,10 +591,6 @@ def __init__(self, name,
         # auto-detected on first use
         self.override = gc3libs.utils.string_to_boolean(override)
 
-        # default is to use $TMPDIR or '/var/tmp' (see
-        # `tempfile.mkftemp`), but we delay the determination of the
-        # correct dir to the submit_job, so that we don't have to use
-        # `transport` right now.
         self.spooldir = spooldir
 
         # Configure transport
@@ -686,42 +682,6 @@ def _init_resource_dir(self):
                 raise
 
 
-    @property
-    def spooldir(self):
-        """
-        Root folder for all working directories of GC3Pie tasks.
-
-        When this backend executes a task, it first creates a temporary
-        subdirectory of this folder, then launches commands in there.
-
-        If not explicitly set (e.g. at construction time), the "spool
-        directory" will be given a default value according to the logic of
-        :meth:`_init_spooldir`:
-
-        * If the remote environment variable ``TMPDIR`` is set and points to an
-          existing directory, that value is used;
-        * otherwise, the hard-coded default ``/var/tmp`` is used instead.
-        """
-        if not self._spooldir:
-            self._init_spooldir()
-        return self._spooldir
-
-    @spooldir.setter
-    def spooldir(self, value):
-        self._spooldir = value
-
-    def _init_spooldir(self):
-        """Set `self.spooldir` to a sensible value."""
-        rc, stdout, stderr = self.transport.execute_command(
-            'cd "{0}" && pwd'.format(gc3libs.Default.SPOOLDIR))
-        if (rc != 0 or stdout.strip() == '' or stdout[0] != '/'):
-            raise gc3libs.exceptions.SpoolDirError("Unable to use {0} for `spooldir` "
-                                                   "on resource {0}. ".format(gc3libs.Default.SPOOLDIR,
-                                                                              self.name
-                                                                              ))
-        else:
-            self.spooldir = stdout.strip()
-
     @property
     def time_cmd(self):
         if not self._time_cmd_ok:

diff --git a/gc3libs/exceptions.py b/gc3libs/exceptions.py
@@ -384,6 +384,15 @@ class OutputNotAvailableError(InvalidOperation):
     pass
 
 
+class SpoolDirError(LRMSError, InvalidValue, UnrecoverableError):
+
+    """
+    Raised when a backend fails to access the spooldir either because
+    it does not exists or cannot be read.
+    """
+    pass
+
+
 class TaskError(Error):
 
     """
@@ -482,13 +491,6 @@ class ApplicationDescriptionError(FatalError):
     """
     pass
 
-class SpoolDirError(InvalidValue, UnrecoverableError):
-
-    """
-    Raised when a backend fails to access the spooldir either because
-    it does not exists or cannot be read.
-    """
-
 
 # main: run tests
 

diff --git a/gc3libs/tests/test_config.py b/gc3libs/tests/test_config.py
@@ -2,7 +2,7 @@
 #
 # -*- coding: utf-8 -*-
 #
-#  Copyright (C) 2010-2012, 2015 S3IT, Zentrale Informatik, University of Zurich
+#  Copyright (C) 2010-2012, 2015, 2018 S3IT, Zentrale Informatik, University of Zurich
 #
 #
 #  This program is free software; you can redistribute it and/or modify it
@@ -621,7 +621,7 @@ def test_pbs_prologue_and_epilogue_contents_when_files(self):
         lrms = [ resource for resource in self.core.get_resources() if resource.name == 'testpbs'][0]
         # Ugly hack. We have to list the job dirs to check which one
         # is the new one.
-        jobdir = os.path.expanduser('{0}'.format(lrms.spooldir))
+        jobdir = os.path.expandvars(lrms.spooldir)
         jobs = []
         if os.path.isdir(jobdir):
             jobs = os.listdir(jobdir)
@@ -671,7 +671,7 @@ def test_pbs_prologue_and_epilogue_contents_when_not_files(self):
         lrms = [ resource for resource in self.core.get_resources() if resource.name == 'testpbs'][0]
         # Ugly hack. We have to list the job dirs to check which one
         # is the new one.
-        jobdir = os.path.expanduser('{0}'.format(lrms.spooldir))
+        jobdir = os.path.expandvars(lrms.spooldir)
         jobs = []
         if os.path.isdir(jobdir):
             jobs = os.listdir(jobdir)