Merge remote-tracking branch 'origin/master' into 0.6.x

* origin/master: (38 commits)
  RF: completely remove custom subclass HTTPError for now
  RF: quit loop even before the first .get if exception was raised
  ENH(UX): set warn to True upon issueing first warning that we got more than expected
  BF: ask for pyout != 0.6.0 so we could keep passing tuples with field names
  RF: dandiapi - remove not used include_metadata option
  ENH: use result.ok instead of checking only for 200 and 201
  BF: close the session if we started a new one
  BF: cast env var DANDI_MAX_CHUNK_SIZE to int
  RF: do not duplicate defaults within kwargs in @click entrypoint for download
  BF(TST): yet one more test fix up for incorrect handling while merging
  BF: moved lock_dandiset into correct position (incorrect merge side effect)
  RF: "dandiapi" -> "api" and "DANDI API" to match /server-info simple "api"
  BF: making linting happy
  Remove duplicate "requests" dependency
  get_instance(): Immediately return known instances that lack redirector URLs
  Raise helpful error if /server-info returns invalid version
  Make redirector report http://localhost:8081 as Girder URL
  Test uploading an unregistered dandiset
  Assert a certain code path is unreachable
  Test that lock_dandiset() is called when uploading
  ...

.github/workflows/test.yml

@@ -25,8 +25,10 @@ jobs:
     steps:
     - name: Set up environment
       uses: actions/checkout@v1
-      with:  # no need for the history
-        fetch-depth: 1
+      with:
+        # Fetch all commits so that versioneer will return something compatible
+        # with semantic-version
+        fetch-depth: 0
     - name: Set up Python ${{ matrix.python }}
       uses: actions/setup-python@v1
       with:

dandi/cli/cmd_download.py

@@ -55,7 +55,7 @@
 )
 @click.argument("url", nargs=-1)
 @map_to_click_exceptions
-def download(url, output_dir, existing, jobs=6, format="pytout"):
+def download(url, output_dir, existing, jobs, format):
     """Download a file or entire folder from DANDI"""
     # First boring attempt at click commands being merely an interface to
     # Python function

dandi/cli/formatter.py

@@ -45,9 +45,10 @@ def __init__(self, out=None):
         self.records = []
 
     def __exit__(self, exc_type, exc_value, traceback):
-        import yaml
+        import ruamel.yaml
 
-        self.out.write(yaml.dump(self.records))
+        yaml = ruamel.yaml.YAML()
+        yaml.dump(self.records, self.out)
 
     def __call__(self, rec):
         self.records.append(rec)

dandi/cli/tests/test_ls.py

@@ -2,6 +2,7 @@
 import pytest
 
 from ..command import ls
+from ...utils import yaml_load
 
 
 @pytest.mark.parametrize("format", ("auto", "json", "json_pp", "yaml", "pyout"))
@@ -17,10 +18,9 @@ def test_smoke(simple1_nwb_metadata, simple1_nwb, format):
 
         load = json.loads
     elif format == "yaml":
-        import yaml
 
         def load(s):
-            obj = yaml.load(s, Loader=yaml.BaseLoader)
+            obj = yaml_load(s, typ="base")
             assert len(obj) == 1  # will be a list with a single elem
             return obj[0]
 

dandi/conftest.py

@@ -1 +1 @@
-from .tests.fixtures import *
+from .tests.fixtures import *  # noqa: F401, F403

dandi/consts.py

@@ -75,9 +75,7 @@
 dandiset_metadata_file = "dandiset.yaml"
 dandiset_identifier_regex = "^[0-9]{6}$"
 
-dandi_instance = namedtuple(
-    "dandi_instance", ("girder", "gui", "redirector", "dandiapi")
-)
+dandi_instance = namedtuple("dandi_instance", ("girder", "gui", "redirector", "api"))
 
 known_instances = {
     "local-girder-only": dandi_instance(
@@ -118,7 +116,7 @@
 # Chunk size when iterating a download (and upload) body. Taken from girder-cli
 # TODO: should we make them smaller for download than for upload?
 # ATM used only in download
-MAX_CHUNK_SIZE = os.environ.get("DANDI_MAX_CHUNK_SIZE", 1024 * 1024 * 8)  # 64
+MAX_CHUNK_SIZE = int(os.environ.get("DANDI_MAX_CHUNK_SIZE", 1024 * 1024 * 8))  # 64
 
 #
 # Some routes

dandi/dandiapi.py

@@ -9,16 +9,6 @@
 lgr = get_logger()
 
 
-class HTTPError(requests.HTTPError):
-    """An HTTP error occurred.
-
-    Following Girder's recommendation of having its HttpError deprecated,
-    this is just a helper to bring that HTTPError into our space
-    """
-
-    pass
-
-
 # Following class is loosely based on GirderClient, with authentication etc
 # being stripped.
 # TODO: add copyright/license info
@@ -56,10 +46,13 @@ def session(self, session=None):
         """
         self._session = session if session else requests.Session()
 
-        yield self._session
-
-        self._session.close()
-        self._session = None
+        try:
+            yield self._session
+        finally:
+            # close only if we started a new one
+            if not session:
+                self._session.close()
+            self._session = None
 
     def _request_func(self, method):
         if self._session is not None:
@@ -141,8 +134,8 @@ def send_request(
         )
 
         # If success, return the json object. Otherwise throw an exception.
-        if result.status_code not in (200, 201):
-            raise HTTPError(
+        if not result.ok:
+            raise requests.HTTPError(
                 f"Error {result.status_code} while sending {method} request to {url}",
                 response=result,
             )
@@ -234,9 +227,7 @@ def get_asset(self, dandiset_id, version, uuid):
     def get_dandiset(self, dandiset_id, version):
         return self.get(f"/dandisets/{dandiset_id}/versions/{version}/")
 
-    def get_dandiset_assets(
-        self, dandiset_id, version, location=None, page_size=None, include_metadata=True
-    ):
+    def get_dandiset_assets(self, dandiset_id, version, location=None, page_size=None):
         """A generator to provide asset records
         """
         if location is not None:
@@ -297,13 +288,11 @@ def get_dandiset_assets(
                     r["modified"] = ensure_datetime(uploaded_mtime)
             yield r
 
-    def get_dandiset_and_assets(
-        self, dandiset_id, version, location=None, include_metadata=True
-    ):
+    def get_dandiset_and_assets(self, dandiset_id, version, location=None):
         """This is pretty much an adapter to provide "harmonized" output in both
-        girder and dandiapi clients.
+        girder and DANDI api clients.
 
-        Harmonization should happen toward "dandiapi" BUT AFAIK it is still influx
+        Harmonization should happen toward DADNDI API BUT AFAIK it is still influx
         """
         # Fun begins!
         location_ = "/" + location if location else ""
@@ -320,9 +309,7 @@ def get_dandiset_and_assets(
         # Get dandiset information
         dandiset = self.get_dandiset(dandiset_id, version)
         # TODO: location
-        assets = self.get_dandiset_assets(
-            dandiset_id, version, location=location, include_metadata=include_metadata
-        )
+        assets = self.get_dandiset_assets(dandiset_id, version, location=location)
         return dandiset, assets
 
     def get_download_file_iter(

dandi/dandiset.py

@@ -1,11 +1,10 @@
 """Classes/utilities for support of a dandiset"""
 
 import os
-import yaml
 from pathlib import Path
 
 from .consts import dandiset_metadata_file
-from .utils import find_parent_directory_containing, yaml_dump
+from .utils import find_parent_directory_containing, yaml_dump, yaml_load
 
 from . import get_logger
 
@@ -42,7 +41,7 @@ def _load_metadata(self):
             with open(self._metadata_file_obj) as f:
                 # TODO it would cast 000001 if not explicitly string into
                 # an int -- we should prevent it... probably with some custom loader
-                self.metadata = yaml.safe_load(f)
+                self.metadata = yaml_load(f, typ="safe")
         else:
             self.metadata = None
 
@@ -70,15 +69,9 @@ def update_metadata(self, meta):
             lgr.debug("No updates to metadata, returning")
             return
 
-        # We will use ruaml to load/save it
-        # Seems to be too tricky to add new entries, etc so we will
-        # just resort to explicitly adding the header while saving
-        # import ruamel.yaml
-        # yaml = ruamel.yaml.YAML()  # defaults to round-trip if no parameters
-        # given
         if self._metadata_file_obj.exists():
             with open(self._metadata_file_obj) as f:
-                rec = yaml.safe_load(f)
+                rec = yaml_load(f, typ="safe")
         else:
             rec = {}