Repository: DataBiosphere/toil

Issue:
FileJobStore's default_caching() result of False is not respected
The `FileJobStore` is supposed to be able to set a default of no caching on the workflow:
https://github.com/DataBiosphere/toil/blob/e45b27cb3086fc57f35653562357ab9537f45d4e/src/toil/jobStores/fileJobStore.py#L70-L78

But this no longer works. If I run a file job store workflow with a batch system that doesn't support caching (like Slurm until #928/#4775 gets fixed), I get a complaint that caching needs to manually be set to off.

This didn't used to happen; I suspect that this is a regression introduced with the config file support.

We should fix `default_caching()` to be respected again, and we should add a test to make sure it stays working.

┆Issue is synchronized with this [Jira Story](https://ucsc-cgl.atlassian.net/browse/TOIL-1490)
┆Issue Number: TOIL-1490


Discussion:

Patch:
diff --git a/src/toil/options/common.py b/src/toil/options/common.py
index 61086f3d..5c041f3b 100644
--- a/src/toil/options/common.py
+++ b/src/toil/options/common.py
@@ -733,4 +733,4 @@ def add_base_toil_options(parser: ArgumentParser, jobstore_as_flag: bool = False
 
     # dest is set to enableCaching to not conflict with the current --caching destination
     caching.add_argument('--disableCaching', dest='enableCaching', action='store_false', help=SUPPRESS)
-    caching.set_defaults(disableCaching=None)
+    caching.set_defaults(enableCaching=None)


Test patch:
diff --git a/src/toil/test/options/__init__.py b/src/toil/test/options/__init__.py
new file mode 100644
index 00000000..471fe7cd
--- /dev/null
+++ b/src/toil/test/options/__init__.py
@@ -0,0 +1,13 @@
+# Copyright (C) 2015-2021 Regents of the University of California
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
diff --git a/src/toil/test/options/options.py b/src/toil/test/options/options.py
new file mode 100644
index 00000000..c3c30749
--- /dev/null
+++ b/src/toil/test/options/options.py
@@ -0,0 +1,37 @@
+from configargparse import ArgParser
+
+from toil.common import addOptions, Toil
+from toil.test import ToilTest
+
+
+class OptionsTest(ToilTest):
+    """
+    Class to test functionality of all Toil options
+    """
+    def test_default_caching_slurm(self):
+        """
+        Test to ensure that caching will be set to false when running on Slurm
+        :return:
+        """
+        parser = ArgParser()
+        addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False)
+        test_args = ["--jobstore=example-jobstore", "--batchSystem=slurm"]
+        options = parser.parse_args(test_args)
+        with Toil(options) as toil:
+            caching_value = toil.config.caching
+        self.assertEqual(caching_value, False)
+
+    def test_caching_option_priority(self):
+        """
+        Test to ensure that the --caching option takes priority over the default_caching() return value
+        :return:
+        """
+        parser = ArgParser()
+        addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False)
+        # the kubernetes batchsystem (and I think all batchsystems including singlemachine) return False
+        # for default_caching
+        test_args = ["--jobstore=example-jobstore", "--batchSystem=kubernetes", "--caching=True"]
+        options = parser.parse_args(test_args)
+        with Toil(options) as toil:
+            caching_value = toil.config.caching
+        self.assertEqual(caching_value, True)