Remove dandi-archive compatibility kludges that no known instance needs

DEVELOPMENT.md

@@ -253,23 +253,35 @@ other instances may lag behind, or run forks of, dandi-archive.
 
 **Support for a new dandi-archive feature must be backward compatible** with
 prior versions of dandi-archive, in particular those currently deployed by the
-DANDI and EMBER instances.  A new dandi-cli release must keep working against
-all of them, and use the new feature only where the server provides it.  This
-way dandi-cli can be released before the feature is released or deployed by
-dandi-archive, and tested against sandbox instances that already have it.  We
-do not need synchronized releases of dandi-cli and dandi-archive, or
-synchronized deployments across all instances: those are fragile and need
+DANDI, EMBER and LINC instances.  A new dandi-cli release must keep working
+against all of them, and use the new feature only where the server provides
+it.  This way dandi-cli can be released before the feature is released or
+deployed by dandi-archive, and tested against sandbox instances that already
+have it.  We do not need synchronized releases of dandi-cli and dandi-archive,
+or synchronized deployments across all instances: those are fragile and need
 orchestration.  Do not ask instances that lack the feature to reject the new
 client via `cli-minimal-version` or `cli-bad-versions` in `/api/info/`.
 
+LINC runs a fork of dandi-archive 0.3.115 (`lincbrain/linc-archive`) with
+schema version 0.6.9, far behind the others, and is still in use.  To see
+what the instances deploy:
+
+    for h in api.dandiarchive.org api.sandbox.dandiarchive.org \
+             api-dandi.emberarchive.org api-dandi.sandbox.emberarchive.org \
+             api.lincbrain.org staging-api.lincbrain.org; do
+        echo "$h: $(curl -s https://$h/api/info/ \
+            | jq -c '{version, schema_version, allowed_schema_versions}')"
+    done
+
 Decide, per server, whether the feature is available, either:
 
 1. **By server version**, if the dandi-archive release that added the feature
    is known: compare it with `DandiAPIClient.server_info["version"]` (the
    `version` field of `/api/info/`).  Forks may report versions that cannot be
-   compared with dandi-archive's (e.g. LINC reported `0.0.post6586+g39387b8`),
-   so a version check works only for instances deploying dandi-archive
-   releases.
+   compared with dandi-archive's (e.g. EMBER reports
+   `0.0.post1.dev7927+g9e4606812` and LINC `0.0.post6586+g39387b8`), so a
+   version check works only for instances deploying dandi-archive releases,
+   i.e. only DANDI's.
 2. **By sensing** the feature, which is the most likely case while it is
    available only on a sandbox instance or in a fork.  Probe the server once,
    or detect support from the response to the first use, remember the result
@@ -281,16 +293,19 @@ Decide, per server, whether the feature is available, either:
 Never decide by the instance name or URL.  In addition:
 
 - In a comment at the conditional, refer to the dandi-archive issue/PR it
-  accommodates and say when the fallback can be removed (e.g. "once LINC and
-  EMBER update their API to match dandi-archive >= 0.12.14"), so that it can
-  be found, reviewed, and removed later.
+  accommodates and say when the fallback can be removed (e.g. "once LINC signs
+  the header for embargoed Zarrs, as dandi-archive >= 0.12.14 does"), so that
+  it can be found, reviewed, and removed later.
 - Log use of the fallback at DEBUG or INFO level, not as a warning or error;
   users cannot do anything about it.
 - Test both code paths.  Integration tests run against the latest
   `dandiarchive/dandiarchive-api` Docker image (dandi-archive `master`), so
   test the prior behavior with mocked responses (`responses`), and skip
   integration tests needing the feature if the server does not support it.
-- Remove the fallback once all known instances support the feature.
+- Remove the fallback once no known instance needs it: all support the
+  feature, or those that do not cannot reach the code (e.g. a server whose
+  schema version is older than the client's never needs
+  `allowed_schema_versions`).
 
 Examples in the code (links are to the 0.81.0 release):
 

dandi/dandiapi.py

@@ -701,8 +701,7 @@ def check_schema_version(self, schema_version: str | None = None) -> None:
           number for 0.x series, and same MAJOR version for/after 1.x series;
         - the same;
         - higher than the client has, but only if the client's schema version is listed
-          among the server's `allowed_schema_versions` (as returned by the `/info` API endpoint),
-          or if not there -- `dandischema.consts.ALLOWED_INPUT_SCHEMAS` is consulted.
+          among the server's `allowed_schema_versions` (as returned by the `/info` API endpoint).
 
         If neither of above, a `SchemaVersionError` is raised.
 
@@ -717,11 +716,10 @@ def check_schema_version(self, schema_version: str | None = None) -> None:
             schema_version
         )
         if server_ver > our_ver:
-            # TODO: potentially adjust here if name would be different: see
-            # https://github.com/dandi/dandi-archive/issues/2624
-            allowed_schema_versions = server_info.get(
-                "allowed_schema_versions", dandischema.consts.ALLOWED_INPUT_SCHEMAS
-            )
+            # A server without the field (dandi-archive < 0.19.0,
+            # https://github.com/dandi/dandi-archive/pull/2625) accepts no older
+            # schema.  LINC is one, but its schema is older than any client's.
+            allowed_schema_versions = server_info.get("allowed_schema_versions") or []
             if schema_version not in allowed_schema_versions:
                 raise SchemaVersionError(
                     f"Server uses schema version {server_schema_version};"

dandi/download.py

@@ -1213,11 +1213,7 @@ def _check_attempts_and_sleep(
         lgr.debug("%s - download failed after %d attempts: %s", path, attempt, exc)
         return 0
     if exc.response is not None:
-        if exc.response.status_code not in (
-            400,  # Bad Request, but happened with girder:
-            # https://github.com/dandi/dandi-cli/issues/87
-            *RETRY_STATUSES,
-        ):
+        if exc.response.status_code not in RETRY_STATUSES:
             lgr.debug(
                 "%s - download failed due to response %d: %s",
                 path,

dandi/files/zarr.py

@@ -601,6 +601,12 @@ def mkzarr() -> str:
                     json={"name": asset_path, "dandiset": dandiset.identifier},
                 )
             except requests.HTTPError as e:
+                # dandi-archive >= 1.0.0 rejects a duplicate with the DRF
+                # "unique set" error
+                # (https://github.com/dandi/dandi-archive/pull/2589), but still
+                # answers "Zarr already exists" if it loses a race to a
+                # concurrent creation, as older versions always do; both must
+                # stay.
                 if e.response is not None and (
                     "Zarr already exists" in e.response.text
                     or "dandiset, name must make a unique set" in e.response.text
@@ -1008,8 +1014,11 @@ def _upload_zarr_file(
         if item.content_type is not None:
             headers["Content-Type"] = item.content_type
 
-        # TODO: Replace below implementation with this once LINC and EMBER
-        # update their API to match dandi-archive >= 0.12.14
+        # TODO: Replace below implementation with this once LINC signs the
+        # header for embargoed Zarrs, as dandi-archive >= 0.12.14 does
+        # (https://github.com/dandi/dandi-archive/pull/2503).  DANDI and EMBER
+        # do; LINC (a fork of dandi-archive 0.3.115) does not, and accepts
+        # embargoed Zarrs without it.
         # if dandiset.embargo_status == EmbargoStatus.EMBARGOED:
         #     # Technically, a header can be specified more than once.
         #     # Join all together just in case.

dandi/tests/test_dandiapi.py

@@ -13,6 +13,7 @@
 import anys
 import click
 import dandischema
+from dandischema.consts import ALLOWED_INPUT_SCHEMAS
 from dandischema.models import UUID_PATTERN, DigestType, get_schema_version
 from packaging.version import Version as _PackagingVersion
 import pytest
@@ -348,6 +349,8 @@ def test_check_schema_version(
 ) -> None:
     server_info = {
         "schema_version": server_schema_version,
+        # as dandi-archive reports it
+        "allowed_schema_versions": ALLOWED_INPUT_SCHEMAS,
         "version": "0.0.0",
         "services": {
             "api": {"url": "https://test.nil/api"},
@@ -391,6 +394,38 @@ def _mock_server_info(schema_version: str) -> None:
     responses.add(responses.GET, "https://test.nil/api/info/", json=info)
 
 
+@pytest.mark.ai_generated
+@pytest.mark.parametrize(
+    "allowed_schema_versions,local_schema_version,should_raise",
+    [
+        (["9.9.8", "9.9.9"], "9.9.8", False),
+        # In the client's own ALLOWED_INPUT_SCHEMAS, but not allowed by the server
+        (["9.9.8", "9.9.9"], "0.6.9", True),
+        # A server not reporting the field (dandi-archive < 0.19.0) allows none
+        (None, "9.9.8", True),
+    ],
+)
+@responses.activate
+def test_check_schema_version_allowed_by_server(
+    allowed_schema_versions: list[str] | None,
+    local_schema_version: str,
+    should_raise: bool,
+) -> None:
+    info = _server_info("9.9.9")
+    if allowed_schema_versions is not None:
+        info["allowed_schema_versions"] = allowed_schema_versions
+    responses.add(responses.GET, "https://test.nil/server-info", json=info)
+    responses.add(responses.GET, "https://test.nil/api/info/", json=info)
+    client = DandiAPIClient("https://test.nil/api")
+    if should_raise:
+        with pytest.raises(
+            SchemaVersionError, match="not among any of the allowed upgradable"
+        ):
+            client.check_schema_version(local_schema_version)
+    else:
+        client.check_schema_version(local_schema_version)
+
+
 @pytest.mark.ai_generated
 @pytest.mark.parametrize(
     "server_schema,obj_schema,expected_schema,dropped,kept",

dandi/tests/test_download.py

@@ -1462,6 +1462,24 @@ def test__check_attempts_and_sleep() -> None:
             assert mock_sleep.call_args.args[0] >= 0
 
 
+@pytest.mark.ai_generated
+@pytest.mark.parametrize("status_code", [400, 403, 404])
+def test__check_attempts_and_sleep_no_retry(status_code: int) -> None:
+    response = requests.Response()
+    response.status_code = status_code
+    with mock.patch("time.sleep") as mock_sleep:
+        assert (
+            _check_attempts_and_sleep(
+                Path("some/path"),
+                HTTPError(response=response),
+                attempt=1,
+                attempts_allowed=2,
+            )
+            == 0
+        )
+        mock_sleep.assert_not_called()
+
+
 @pytest.mark.parametrize("status_code", [429, 503])
 def test__check_attempts_and_sleep_retries(status_code: int) -> None:
     f = partial(_check_attempts_and_sleep, Path("some/path"))

dandi/utils.py

@@ -576,11 +576,6 @@ def get_instance(dandi_instance_id: str | DandiInstance) -> DandiInstance:
     else:
         dandi_id = dandi_instance_id
         instance = known_instances[dandi_id]
-    if dandi_id == "dandi-staging":
-        raise ValueError(
-            "'dandi-staging' DANDI instance identifier was removed. "
-            "The instance was renamed into 'dandi-sandbox', please use that identifier instead."
-        )
     if redirector_url is None:
         assert instance is not None
         return _get_instance(instance.api.rstrip("/"), True, instance, dandi_id)