fix: default TEMPDIR to system temp so S3 works with read-only CWD

metaflow/metaflow_config.py

@@ -1,5 +1,6 @@
 import os
 import sys
+import tempfile
 import types
 import uuid
 import datetime
@@ -176,7 +177,7 @@
     ),
 )
 
-TEMPDIR = from_conf("TEMPDIR", ".")
+TEMPDIR = from_conf("TEMPDIR", tempfile.gettempdir())
 
 DATATOOLS_CLIENT_PARAMS = from_conf("DATATOOLS_CLIENT_PARAMS", {})
 if S3_ENDPOINT_URL:

metaflow/plugins/aws/batch/batch_decorator.py

@@ -299,8 +299,9 @@ def task_pre_step(
         self.metadata = metadata
         self.task_datastore = task_datastore
 
-        # current.tempdir reflects the value of METAFLOW_TEMPDIR (the current working
-        # directory by default), or the value of tmpfs_path if tmpfs_tempdir=False.
+        # current.tempdir reflects the value of METAFLOW_TEMPDIR (the system
+        # temporary directory from tempfile.gettempdir() by default), or the
+        # value of tmpfs_path if tmpfs_tempdir=False.
         if not self.attributes["tmpfs_tempdir"]:
             current._update_env({"tempdir": self.attributes["tmpfs_path"]})
 

metaflow/plugins/datatools/s3/s3.py

@@ -479,8 +479,9 @@ class S3(object):
     data = [obj.blob for obj in s3.get_many(urls)]
     s3.close()
     ```
-    You can customize the location of the temporary directory with `tmproot`. It
-    defaults to the current working directory.
+    You can customize the location of the temporary directory with `tmproot`, or
+    with the `METAFLOW_TEMPDIR` configuration variable. If neither is specified,
+    the system temporary directory returned by `tempfile.gettempdir()` is used.
 
     To make it easier to deal with object locations, the client can be initialized
     with an S3 path prefix. There are three ways to handle locations:
@@ -499,7 +500,7 @@ class S3(object):
 
     Parameters
     ----------
-    tmproot : str, default '.'
+    tmproot : str, default METAFLOW_TEMPDIR or tempfile.gettempdir()
         Where to store the temporary directory.
     bucket : str, optional, default None
         Override the bucket from `DATATOOLS_S3ROOT` when `run` is specified.

metaflow/plugins/kubernetes/kubernetes_decorator.py

@@ -529,8 +529,9 @@ def task_pre_step(
         self.metadata = metadata
         self.task_datastore = task_datastore
 
-        # current.tempdir reflects the value of METAFLOW_TEMPDIR (the current working
-        # directory by default), or the value of tmpfs_path if tmpfs_tempdir=False.
+        # current.tempdir reflects the value of METAFLOW_TEMPDIR (the system
+        # temporary directory from tempfile.gettempdir() by default), or the
+        # value of tmpfs_path if tmpfs_tempdir=False.
         if not self.attributes["tmpfs_tempdir"]:
             current._update_env({"tempdir": self.attributes["tmpfs_path"]})
 

test/unit/test_s3_readonly_cwd.py

@@ -0,0 +1,117 @@
+"""Test that S3 client works when current directory is not writeable."""
+
+import os
+from pathlib import Path
+import subprocess
+import sys
+import textwrap
+
+
+S3_SUBPROCESS_SCRIPT = textwrap.dedent(
+    """
+    import os
+    import sys
+    import tempfile
+    from types import ModuleType
+
+    # Keep this test independent of the optional boto3 dependency.
+    boto3 = ModuleType("boto3")
+    boto3_s3 = ModuleType("boto3.s3")
+    transfer = ModuleType("boto3.s3.transfer")
+
+    class TransferConfig:
+        multipart_threshold = 8 * 1024 * 1024
+
+    transfer.TransferConfig = TransferConfig
+    boto3.s3 = boto3_s3
+    boto3_s3.transfer = transfer
+    sys.modules.update(
+        {
+            "boto3": boto3,
+            "boto3.s3": boto3_s3,
+            "boto3.s3.transfer": transfer,
+        }
+    )
+
+    import metaflow
+    from metaflow.plugins.datatools import S3 as DatatoolsS3
+    from metaflow.plugins.datatools.s3 import S3
+    from metaflow.plugins.datastores.s3_storage import S3 as StorageS3
+
+    assert metaflow.S3 is DatatoolsS3 is S3 is StorageS3
+
+    s3_client = S3()
+    try:
+        expected_tempdir = os.environ.get("TEST_EXPECTED_METAFLOW_TEMPDIR")
+        if expected_tempdir is None:
+            assert s3_client._tmproot == tempfile.gettempdir()
+            assert s3_client._tmpdir.startswith(tempfile.gettempdir())
+            assert not s3_client._tmpdir.startswith(os.getcwd())
+        else:
+            assert s3_client._tmproot == expected_tempdir
+            assert os.path.dirname(s3_client._tmpdir) == expected_tempdir
+    finally:
+        s3_client.close()
+    """
+)
+
+
+def _run_s3_subprocess(cwd, env, expected_tempdir=None):
+    """Run an S3 configuration scenario in a fresh Python interpreter."""
+    repo_root = str(Path(__file__).resolve().parents[2])
+    env = env.copy()
+    env["PYTHONPATH"] = os.pathsep.join(
+        path for path in (repo_root, env.get("PYTHONPATH")) if path
+    )
+    if expected_tempdir is not None:
+        env["TEST_EXPECTED_METAFLOW_TEMPDIR"] = str(expected_tempdir)
+    else:
+        env.pop("TEST_EXPECTED_METAFLOW_TEMPDIR", None)
+
+    import metaflow
+
+    s3_class_before = metaflow.S3
+    result = subprocess.run(
+        [sys.executable, "-c", S3_SUBPROCESS_SCRIPT],
+        cwd=str(cwd),
+        env=env,
+        capture_output=True,
+        text=True,
+        check=False,
+        timeout=30,
+    )
+    assert (
+        result.returncode == 0
+    ), "S3 subprocess failed with exit code %d\nstdout:\n%s\nstderr:\n%s" % (
+        result.returncode,
+        result.stdout,
+        result.stderr,
+    )
+    assert metaflow.S3 is s3_class_before
+
+
+def test_s3_client_readonly_cwd(tmp_path):
+    """Test that S3 client doesn't require writable CWD (issue #854)."""
+    readonly_dir = tmp_path / "readonly"
+    readonly_dir.mkdir()
+    os.chmod(readonly_dir, 0o555)
+
+    env = os.environ.copy()
+    env.pop("METAFLOW_TEMPDIR", None)
+    env["METAFLOW_HOME"] = str(tmp_path / "empty_metaflow_home")
+    try:
+        _run_s3_subprocess(readonly_dir, env)
+    finally:
+        # Make directory writable again so pytest can clean it up.
+        os.chmod(readonly_dir, 0o755)
+
+
+def test_s3_client_respects_metaflow_tempdir(tmp_path):
+    """Test that METAFLOW_TEMPDIR overrides the system temporary directory."""
+    configured_dir = tmp_path / "configured"
+    configured_dir.mkdir()
+
+    env = os.environ.copy()
+    env["METAFLOW_TEMPDIR"] = str(configured_dir)
+    env["METAFLOW_HOME"] = str(tmp_path / "empty_metaflow_home")
+    _run_s3_subprocess(tmp_path, env, configured_dir)