various fixes for s3fs use (#1312)

* various fixes for s3fs use

* lint

server/app/app.py

@@ -98,7 +98,7 @@ def dataroot_test_index():
     data += "<body><H1>Welcome to cellxgene</H1>"
 
     config = current_app.app_config
-    locator = DataLocator(config.multi_dataset__dataroot)
+    locator = DataLocator(config.multi_dataset__dataroot, config=config)
     datasets = []
     for fname in locator.ls():
         location = path_join(config.multi_dataset__dataroot, fname)

server/common/app_config.py

@@ -93,6 +93,8 @@ class AppConfig(object):
             self.diffexp__enable = dc["diffexp"]["enable"]
             self.diffexp__lfc_cutoff = dc["diffexp"]["lfc_cutoff"]
 
+            self.data_locator__s3__region_name = dc["data_locator"]["s3"]["region_name"]
+
             self.adaptor__cxg_adaptor__tiledb_ctx = dc["adaptor"]["cxg_adaptor"]["tiledb_ctx"]
             self.adaptor__anndata_adaptor__backed = dc["adaptor"]["anndata_adaptor"]["backed"]
 

server/common/data_locator.py

@@ -25,7 +25,7 @@ class DataLocator:
 
     """
 
-    def __init__(self, uri_or_path):
+    def __init__(self, uri_or_path, app_config=None):
         if isinstance(uri_or_path, DataLocator):
             locator = uri_or_path
             self.uri_or_path = locator.uri_or_path
@@ -38,8 +38,17 @@ class DataLocator:
             # work-around for LocalFileSystem not treating file: and None as the same scheme/protocol
             self.cname = self.path if self.protocol == "file" else self.uri_or_path
 
-        # will throw RuntimeError if the protocol is unsupported
+        # fsspec.filesystem will throw RuntimeError if the protocol is unsupported
-        self.fs = fsspec.filesystem(self.protocol)
+        if self.protocol == "s3" and app_config.data_locator__s3__region_name:
+            self.fs = fsspec.filesystem(
+                self.protocol, config_kwargs={"region_name": app_config.data_locator__s3__region_name}
+            )
+        else:
+            self.fs = fsspec.filesystem(self.protocol)
+
+    def __repr__(self):
+        return f"DataLocator(protocol={self.protocol}, cname={self.cname}, "
+        f"path={self.path}, uri_or_path={self.uri_or_path})"
 
     @staticmethod
     def _get_protocol_and_path(uri_or_path):

server/common/default_config.py

@@ -60,6 +60,10 @@ diffexp:
   enable: true
   lfc_cutoff: 0.01
 
+data_locator:
+  s3:
+    region_name: us-east-1
+
 adaptor:
   cxg_adaptor:
     tiledb_ctx:

server/common/health.py

@@ -5,12 +5,12 @@ from server import __version__ as cellxgene_version
 from server.common.data_locator import DataLocator
 
 
-def _is_accessible(path):
+def _is_accessible(path, config):
     if path is None:
         return True
 
     try:
-        dl = DataLocator(path)
+        dl = DataLocator(path, config)
         return dl.exists()
     except RuntimeError:
         return False
@@ -25,8 +25,8 @@ def health_check(config):
 
     checks = [
         (config.single_dataset__datapath is not None or config.multi_dataset__dataroot is not None),
-        _is_accessible(config.single_dataset__datapath),
+        _is_accessible(config.single_dataset__datapath, config),
-        _is_accessible(config.multi_dataset__dataroot),
+        _is_accessible(config.multi_dataset__dataroot, config),
     ]
     health["status"] = "pass" if all(checks) else "fail"
     code = HTTPStatus.OK if health["status"] == "pass" else HTTPStatus.BAD_REQUEST

server/data_common/matrix_loader.py

@@ -159,7 +159,7 @@ class MatrixDataType(Enum):
 class MatrixDataLoader(object):
     def __init__(self, location, matrix_data_type=None, app_config=None):
         """ location can be a string or DataLocator """
-        self.location = DataLocator(location)
+        self.location = DataLocator(location, app_config)
         if not self.location.exists():
             raise DatasetAccessError("Dataset does not exist.")
 

server/requirements.txt

@@ -17,4 +17,5 @@ PyYAML>=5.3
 scipy>=1.3.0
 requests>=2.22.0
 tiledb>=0.5.3
-s3fs>=0.4.0
+# s3fs, issue 313 breaks cellxgene
+s3fs==0.4.0