Merge pull request #832 from dcs4cop/forman-828-fix_module_loading_from_s3

Fix module loading from S3

Author: forman

CHANGES.md

@@ -1,5 +1,12 @@
 ## Changes in 1.0.1 (in development)
 
+### Fixes
+
+* Fixed recurring issue where xcube server was unable to locate Python
+  code downloaded from S3 when configuring dynamically computed dataset
+  (configuration `FileSystem: memory`) or augmenting existing datasets 
+  by dynamically computed variables (configuration `Augmentation`). (#828)
+
 
 ## Changes in 1.0.0 
 

test/core/byoa/test_fileset.py

@@ -1,20 +1,18 @@
 import os
 import os.path
-import os.path
 import unittest
+import zipfile
+
+import fsspec
 
-from xcube.core.byoa.constants import TEMP_FILE_PREFIX
 from xcube.core.byoa import FileSet
-import os
-import os.path
-import zipfile
+from xcube.core.byoa.constants import TEMP_FILE_PREFIX
 from xcube.core.dsio import rimraf
 
 PARENT_DIR = os.path.dirname(__file__)
 
 
 class FileSetToLocalTest(unittest.TestCase):
-
     test_dir = os.path.join(os.path.dirname(__file__), 'test-data')
 
     def setUp(self) -> None:
@@ -47,6 +45,23 @@ def test_to_local_from_local_dir(self):
         local_file_set = file_set.to_local()
         self.assertIs(local_file_set, file_set)
 
+    def test_to_local_from_memory_dir(self):
+        mem_fs: fsspec.AbstractFileSystem = fsspec.filesystem("memory")
+        mem_fs.mkdir("package")
+        mem_fs.touch("package/__init__.py")
+        mem_fs.touch("package/module1.py")
+        mem_fs.mkdir("package/module2")
+        mem_fs.touch("package/module2/__init__.py")
+        file_set = FileSet("memory://package")
+        local_file_set = file_set.to_local()
+        self.assertTrue(os.path.isdir(local_file_set.path))
+        self.assertTrue(os.path.isfile(local_file_set.path
+                                       + "/__init__.py"))
+        self.assertTrue(os.path.isfile(local_file_set.path
+                                       + "/module1.py"))
+        self.assertTrue(os.path.isfile(local_file_set.path
+                                       + "/module2/__init__.py"))
+
     def test_to_local_from_local_flat_zip(self):
         zip_path = self._make_zip()
         file_set = FileSet(zip_path)

xcube/core/byoa/fileset.py

@@ -257,15 +257,38 @@ def to_local(self) -> 'FileSet':
         if fs.isdir(root):
             temp_dir = new_temp_dir(prefix=TEMP_FILE_PREFIX,
                                     suffix=temp_file_suffix)
-            # Note, AbstractFileSystem.get() behaves
-            # unpredictably. Sometimes it will create a new directory
-            # named after root in temp_dir (e.g. this is the case
-            # if temp_dir exists). To avoid this, we add a non-existing
-            # directory here:
-            temp_dir += "/data"
-            # TODO: replace by loop so we can apply includes/excludes
-            #   before downloading actual files. See impl of fs.get().
-            fs.get(root + "/*", temp_dir, recursive=True)
+
+            # Note, actually want to use
+            #    fs.get(root + "/*", temp_dir, recursive=True)
+            # here, but fs.get() behaves unpredictably with
+            # fsspec=2023.3.0 and s3fs=2023.3.0.
+            # Sometimes it will create a new subdirectory in temp_dir
+            # named after last path element of root, sometimes not.
+            # Behaviour randomly changes if
+            #   - temp_dir exists or not
+            #   - root or temp_dir paths end with a slash
+            #   - whether root or temp_dir are absolute path or not.
+            #
+            # See https://github.com/dcs4cop/xcube/issues/828
+            #
+            # Workaround used here is to manually download the directory.
+
+            def get_files(source: str, target: str):
+                source_items = fs.listdir(source, detail=True)
+                for source_item in source_items:
+                    source_path = source_item["name"]
+                    source_type = source_item["type"]
+                    source_name = source_path.replace("\\", "/") \
+                        .split("/")[-1]
+                    target_path = os.path.join(target, source_name)
+                    if source_type == "file":
+                        fs.get_file(source_path, target_path)
+                    elif source_type == "directory":
+                        os.mkdir(target_path)
+                        get_files(source_path, target_path)
+
+            get_files(root, temp_dir)
+
             return FileSet(temp_dir,
                            sub_path=self.sub_path,
                            includes=self.includes,