Record the actual NWB Inspector version in validation results

dandi/files/bases.py

@@ -766,42 +766,53 @@ def _check_required_fields(
     return errors
 
 
-_current_nwbinspector_version: str = ""
+_current_nwbinspector_version: Version | None = None
+_nwbinspector_version_lock = Lock()
 
 
-def _get_nwb_inspector_version():
+def _get_nwb_inspector_version() -> Version:
     # Avoid heavy import by importing within function:
     from nwbinspector.utils import get_package_version
 
     global _current_nwbinspector_version
-    if _current_nwbinspector_version is not None:
-        return _current_nwbinspector_version
-    _current_nwbinspector_version = get_package_version(name="nwbinspector")
-    # Ensure latest version of NWB Inspector is installed and used client-side
-    try:
-        from etelemetry import get_project
-
-        max_version = Version(
-            get_project(repo="NeurodataWithoutBorders/nwbinspector")["version"]
-        )
+    # Assets may be validated concurrently (e.g. by `dandi upload`), so lock to
+    # look up the version (and warn if it is outdated) only once per process
+    with _nwbinspector_version_lock:
+        if _current_nwbinspector_version is not None:
+            return _current_nwbinspector_version
+        _current_nwbinspector_version = get_package_version(name="nwbinspector")
+        # Ensure latest version of NWB Inspector is installed and used client-side
+        try:
+            max_version = _get_latest_nwb_inspector_version()
+            if max_version is not None and _current_nwbinspector_version < max_version:
+                lgr.warning(
+                    "NWB Inspector version %s is installed - please "
+                    "use the latest release of the NWB Inspector (%s) "
+                    "when performing `dandi validate`. To update, run "
+                    "`pip install -U nwbinspector` if you installed it with `pip`.",
+                    _current_nwbinspector_version,
+                    max_version,
+                )
 
-        if _current_nwbinspector_version < max_version:
+        except Exception as e:  # In case of no internet connection or other error
             lgr.warning(
-                "NWB Inspector version %s is installed - please "
-                "use the latest release of the NWB Inspector (%s) "
-                "when performing `dandi validate`. To update, run "
-                "`pip install -U nwbinspector` if you installed it with `pip`.",
-                _current_nwbinspector_version,
-                max_version,
+                "Failed to retrieve NWB Inspector version due to %s: %s",
+                type(e).__name__,
+                str(e),
             )
+        return _current_nwbinspector_version
 
-    except Exception as e:  # In case of no internet connection or other error
-        lgr.warning(
-            "Failed to retrieve NWB Inspector version due to %s: %s",
-            type(e).__name__,
-            str(e),
-        )
-    return _current_nwbinspector_version
+
+def _get_latest_nwb_inspector_version() -> Version | None:
+    """
+    Return the latest released version of NWB Inspector according to
+    etelemetry, or `None` if etelemetry checks are disabled via ``NO_ET``
+    """
+    from etelemetry import get_project
+
+    if (project := get_project(repo="NeurodataWithoutBorders/nwbinspector")) is None:
+        return None
+    return Version(project["version"])
 
 
 def _pydantic_errors_to_validation_results(

dandi/tests/fixtures.py

@@ -71,6 +71,17 @@ def capture_all_logs(caplog: pytest.LogCaptureFixture) -> None:
     caplog.set_level(logging.DEBUG, logger="dandi")
 
 
+@pytest.fixture(autouse=True)
+def no_nwbinspector_etelemetry(monkeypatch: pytest.MonkeyPatch) -> None:
+    """
+    Do not ask etelemetry for the latest NWB Inspector release when validating
+    NWB files, to avoid network access and "please upgrade" warnings in tests
+    """
+    monkeypatch.setattr(
+        "dandi.files.bases._get_latest_nwb_inspector_version", lambda: None
+    )
+
+
 # TODO: move into some common fixtures.  We might produce a number of files
 #       and also carry some small ones directly in git for regression testing
 @pytest.fixture(scope="session")

dandi/tests/test_files.py

@@ -1,13 +1,18 @@
 from __future__ import annotations
 
+from importlib.metadata import version
+import json
 from operator import attrgetter
 import os
 from pathlib import Path
 import subprocess
+import sys
+from typing import Any
 from unittest.mock import ANY
 
 from dandischema.models import get_schema_version
 import numpy as np
+from packaging.version import Version
 import pytest
 import zarr
 
@@ -31,6 +36,7 @@
     dandi_file,
     find_dandi_files,
 )
+from ..validate._types import Validator
 
 lgr = get_logger()
 
@@ -416,6 +422,66 @@ def test_validate_bogus(tmp_path):
     # check for two different patterns.
 
 
+@pytest.mark.ai_generated
+def test_validate_nwb_records_nwbinspector_version(simple4_nwb: Path) -> None:
+    errors = dandi_file(simple4_nwb).get_validation_errors()
+    nwbi_errors = [e for e in errors if e.origin.validator is Validator.nwbinspector]
+    # simple4_nwb has a data orientation best practice violation
+    assert nwbi_errors
+    assert {e.origin.validator_version for e in nwbi_errors} == {
+        str(Version(version("nwbinspector")))
+    }
+
+
+# Run in a fresh interpreter so that the module-level version cache starts out
+# in its initial state, as it does for `dandi validate`.
+GET_NWBI_VERSION_SCRIPT = """\
+import json
+import sys
+
+import etelemetry
+
+from dandi.files.bases import _get_nwb_inspector_version
+
+lookups = []
+
+
+def get_project(repo, **kwargs):
+    lookups.append(repo)
+    return json.loads(sys.argv[1])
+
+
+etelemetry.get_project = get_project
+versions = [str(_get_nwb_inspector_version()) for _ in range(2)]
+print(json.dumps({"versions": versions, "lookups": lookups}))
+"""
+
+
+@pytest.mark.ai_generated
+@pytest.mark.parametrize(
+    "project,outdated_warnings",
+    [
+        # What etelemetry.get_project() returns when NO_ET is set
+        (None, 0),
+        ({"version": "9999.0.0"}, 1),
+    ],
+)
+def test_get_nwb_inspector_version_cached(
+    project: dict[str, Any] | None, outdated_warnings: int
+) -> None:
+    r = subprocess.run(
+        [sys.executable, "-c", GET_NWBI_VERSION_SCRIPT, json.dumps(project)],
+        capture_output=True,
+        text=True,
+        check=True,
+    )
+    result = json.loads(r.stdout.splitlines()[-1])
+    assert result["versions"] == [str(Version(version("nwbinspector")))] * 2
+    assert result["lookups"] == ["NeurodataWithoutBorders/nwbinspector"]
+    assert r.stderr.count("please use the latest release") == outdated_warnings
+    assert "Failed to retrieve" not in r.stderr
+
+
 def test_upload_zarr(new_dandiset, tmp_path):
     filepath = tmp_path / "example.zarr"
     zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1))