Merge pull request #1906 from braingram/native_fsspec

Use fsspec for all non-local urls

Author: braingram

.github/workflows/ci.yml

@@ -125,3 +125,13 @@ jobs:
       default_python: '3.11'
       envs: |
         - linux: compatibility
+
+  mocks3:
+    needs: [core]
+    uses: OpenAstronomy/github-actions-workflows/.github/workflows/tox.yml@v1
+    with:
+      submodules: false
+      # Any env name which does not start with `pyXY` will use this Python version.
+      default_python: '3.11'
+      envs: |
+        - linux: mocks3

asdf/_tests/test_generic_io.py

@@ -4,13 +4,15 @@
 import stat
 import sys
 import urllib.request as urllib_request
+from contextlib import nullcontext
 
 import numpy as np
 import pytest
 
 import asdf
 from asdf import exceptions, generic_io
 from asdf.config import config_context
+from asdf.exceptions import AsdfDeprecationWarning
 
 from . import _helpers as helpers
 from . import create_large_tree, create_small_tree
@@ -21,6 +23,26 @@ def tree(request):
     return request.param()
 
 
+@pytest.fixture(params=[True, False])
+def has_fsspec(request, monkeypatch):
+    if request.param:
+        yield True
+    else:
+        pytest.importorskip("fsspec")
+        monkeypatch.setitem(sys.modules, "fsspec", None)
+        yield False
+
+
+@pytest.fixture()
+def warn_no_fsspec(has_fsspec):
+    if has_fsspec:
+        yield nullcontext()
+    else:
+        yield pytest.warns(
+            AsdfDeprecationWarning, match=r"Opening http urls without fsspec is deprecated. Please install fsspec"
+        )
+
+
 def _roundtrip(tree, get_write_fd, get_read_fd, write_options=None, read_options=None):
     write_options = {} if write_options is None else write_options
     read_options = {} if read_options is None else read_options
@@ -259,7 +281,7 @@ def get_read_fd():
 
 
 @pytest.mark.remote_data()
-def test_http_connection(tree, httpserver):
+def test_http_connection(tree, httpserver, warn_no_fsspec):
     path = os.path.join(httpserver.tmpdir, "test.asdf")
 
     def get_write_fd():
@@ -274,9 +296,10 @@ def get_read_fd():
         fd.read(0)
         return fd
 
-    with _roundtrip(tree, get_write_fd, get_read_fd) as ff:
-        assert len(ff._blocks.blocks) == 2
-        assert (ff.tree["science_data"] == tree["science_data"]).all()
+    with warn_no_fsspec:
+        with _roundtrip(tree, get_write_fd, get_read_fd) as ff:
+            assert len(ff._blocks.blocks) == 2
+            assert (ff.tree["science_data"] == tree["science_data"]).all()
 
 
 def test_exploded_filesystem(tree, tmp_path):
@@ -313,7 +336,7 @@ def get_read_fd():
 
 
 @pytest.mark.remote_data()
-def test_exploded_http(tree, httpserver):
+def test_exploded_http(tree, httpserver, warn_no_fsspec):
     path = os.path.join(httpserver.tmpdir, "test.asdf")
 
     def get_write_fd():
@@ -322,8 +345,9 @@ def get_write_fd():
     def get_read_fd():
         return generic_io.get_file(httpserver.url + "test.asdf")
 
-    with _roundtrip(tree, get_write_fd, get_read_fd, write_options={"all_array_storage": "external"}) as ff:
-        assert len(list(ff._blocks.blocks)) == 0
+    with warn_no_fsspec:
+        with _roundtrip(tree, get_write_fd, get_read_fd, write_options={"all_array_storage": "external"}) as ff:
+            assert len(list(ff._blocks.blocks)) == 0
 
 
 def test_exploded_stream_write(small_tree):
@@ -378,7 +402,7 @@ def test_open_stdout():
         pass
 
 
-def test_invalid_obj(tmp_path):
+def test_invalid_obj(tmp_path, has_fsspec):
     with pytest.raises(ValueError, match=r"Can't handle .* as a file for mode 'r'"):
         generic_io.get_file(42)
 
@@ -392,8 +416,14 @@ def test_invalid_obj(tmp_path):
     ):
         generic_io.get_file(fd, "r")
 
-    with pytest.raises(ValueError, match=r"HTTP connections can not be opened for writing"):
-        generic_io.get_file("http://www.google.com", "w")
+    url = "http://www.google.com"
+    mode = "w"
+    if has_fsspec:
+        raises_ctx = pytest.raises(ValueError, match=f"Unable to open {url} with mode {mode}")
+    else:
+        raises_ctx = pytest.raises(ValueError, match=r"HTTP connections can not be opened for writing")
+    with raises_ctx:
+        generic_io.get_file(url, mode)
 
     with pytest.raises(TypeError, match=r"io.StringIO objects are not supported.  Use io.BytesIO instead."):
         generic_io.get_file(io.StringIO())

asdf/generic_io.py

@@ -14,14 +14,15 @@
 import re
 import sys
 import tempfile
+import warnings
 from os import SEEK_CUR, SEEK_END, SEEK_SET
 from urllib.request import url2pathname, urlopen
 
 import numpy as np
 
 from . import util
 from ._extern import atomicfile
-from .exceptions import DelimiterNotFoundError
+from .exceptions import AsdfDeprecationWarning, DelimiterNotFoundError
 from .util import _patched_urllib_parse
 
 __all__ = ["get_file", "get_uri", "resolve_uri", "relative_uri"]
@@ -402,7 +403,8 @@ def close(self):
         """
         if self._close:
             self._fd.close()
-            self._fix_permissions()
+            if hasattr(self, "_fix_permissions"):
+                self._fix_permissions()
 
     def truncate(self, size=None):
         """
@@ -1104,16 +1106,10 @@ def get_file(init, mode="r", uri=None, close=False):
 
     if isinstance(init, (str, pathlib.Path)):
         parsed = _patched_urllib_parse.urlparse(str(init))
-        if parsed.scheme in ["http", "https"]:
-            if "w" in mode:
-                msg = "HTTP connections can not be opened for writing"
-                raise ValueError(msg)
 
-            return _http_to_temp(init, mode, uri=uri)
+        realmode = "r+b" if mode == "rw" else mode + "b"
 
         if parsed.scheme in _local_file_schemes:
-            realmode = "r+b" if mode == "rw" else mode + "b"
-
             # if paths have an extra leading '/' urlparse will
             # parse them even though they violate rfc8089. This will
             # lead to errors or writing files to unexpected locations
@@ -1143,6 +1139,31 @@ def get_file(init, mode="r", uri=None, close=False):
 
             return RealFile(fd, mode, close=True, uri=uri)
 
+        # this is not a local file, import fsspec (if available)
+        try:
+            import fsspec
+        except ImportError:
+            fsspec = None
+
+        if fsspec:
+            fd = fsspec.open(init, realmode)
+            try:
+                fd = fd.__enter__()
+            except NotImplementedError as err:
+                msg = f"Unable to open {init} with mode {mode}"
+                raise ValueError(msg) from err
+            return get_file(fd, uri=uri or init, close=True)
+
+        # finally, allow the legacy http code to handle this (with a warning)
+        if parsed.scheme in ["http", "https"]:
+            if "w" in mode:
+                msg = "HTTP connections can not be opened for writing"
+                raise ValueError(msg)
+
+            msg = "Opening http urls without fsspec is deprecated. Please install fsspec[http]"
+            warnings.warn(msg, AsdfDeprecationWarning)
+            return _http_to_temp(init, mode, uri=uri)
+
     if isinstance(init, io.BytesIO):
         return MemoryIO(init, mode, uri=uri)
 

changes/1906.feature.rst

@@ -0,0 +1 @@
+Optionally use fsspec for urls (like those for s3 resources) provided to asdf.open.

changes/1906.removal.rst

@@ -0,0 +1 @@
+Deprecate opening http uris unless fsspec is installed.