From 85bf738ab4eb615164735010d7b3d4bb3a31bca0 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Mon, 27 Oct 2025 13:38:53 -0700 Subject: [PATCH 01/11] feat: obtain API key for DANDI instance from corresponding env var Instead of attempting to obtain API key from the environment variable DANDI_API_KEY when connecting to any known DANI instances, obtain the API key from an environment variable corresponding the to name of the instance. For example, obtain the key from the `DANDI_API_KEY` env var when connecting to the known DANI instance named `dandi` and obtain the key from the `EMBER_SANDBOX_API_KEY` var when the connecting to the known instance named `ember-sandbox`. I.e., the environment variable name is the capitalized version of the instance's name with "-" replaced by "_" suffixed by "_API_KEY". --- dandi/dandiapi.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dandi/dandiapi.py b/dandi/dandiapi.py index 5834b7513..1d3c1d48b 100644 --- a/dandi/dandiapi.py +++ b/dandi/dandiapi.py @@ -486,10 +486,11 @@ def authenticate(self, token: str, save_to_keyring: bool = False) -> None: def dandi_authenticate(self) -> None: """ Acquire and set the authentication token/API key used by the - `DandiAPIClient`. If the :envvar:`DANDI_API_KEY` environment variable - is set, its value is used as the token. Otherwise, the token is looked + `DandiAPIClient`. + If the :envvar:`f"{self._instance_id.replace('-', '_')}_API_KEY"` environment + variable is set, its value is used as the token. Otherwise, the token is looked up in the user's keyring under the service - ":samp:`dandi-api-{INSTANCE_NAME}`" [#auth]_ and username "``key``". + ":samp:`dandi-api-{self.dandi_instance.name}`" [#auth]_ and username "``key``". If no token is found there, the user is prompted for the token, and, if it proves to be valid, it is stored in the user's keyring. @@ -497,9 +498,10 @@ def dandi_authenticate(self) -> None: "``dandi-api-dandi-sandbox``" for the sandbox server """ # Shortcut for advanced folks - api_key = os.environ.get("DANDI_API_KEY", None) + env_var_name = f"{self._instance_id.replace('-', '_')}_API_KEY" + api_key = os.environ.get(env_var_name, None) if api_key: - lgr.debug("Using api key from DANDI_API_KEY environment variable") + lgr.debug(f"Using `{env_var_name}` environment variable as the API key") self.authenticate(api_key) return client_name, app_id = self._get_keyring_ids() From 8170d360975e85073bb57fc7d70e641a54b17a6e Mon Sep 17 00:00:00 2001 From: Isaac To Date: Mon, 27 Oct 2025 15:01:03 -0700 Subject: [PATCH 02/11] docs: adjust documentation --- DEVELOPMENT.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index df49e91e7..624b841cb 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -55,8 +55,17 @@ development command line options. otherwise be hidden from the user-visible (`--help`) interface, unless this env variable is set to a non-empty value -- `DANDI_API_KEY` -- avoids using keyrings, thus making it possible to - "temporarily" use another account etc for the "API" version of the server. +- `{CAPITALIZED_INSTANCE_NAME_WITH_UNDERSCORE}_API_KEY` -- + Provides the API key to access a known DANDI instance. + Respective keys for multiple instances can be provided. The name of the environment + variable providing the key for a specific known DANDI instance corresponds to the name + of the instance. For example, the environment variable `DANDI_API_KEY` provides the key + for the known instance named `dandi` and the environment variable + `EMBER_SANDBOX_API_KEY` provides the key for the known instance named `ember-sandbox`. + I.e., the environment variable name is the capitalized version of the instance's name + with "-" replaced by "_" suffixed by "_API_KEY". Providing API through environment + variables avoids using keyrings, thus making it possible to "temporarily" use another + account etc for the "API" version of the server. - `DANDI_LOG_LEVEL` -- set log level. By default `INFO`, should be an int (`10` - `DEBUG`). From 5a21bd6e9db2f344d5ca81f12f1b6bce193ddbf6 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Tue, 28 Oct 2025 13:03:23 -0700 Subject: [PATCH 03/11] feat: def utility function to get env var name for API key from a `DandiInstance` --- dandi/dandiapi.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/dandi/dandiapi.py b/dandi/dandiapi.py index 1d3c1d48b..86c00b57c 100644 --- a/dandi/dandiapi.py +++ b/dandi/dandiapi.py @@ -77,6 +77,17 @@ class VersionStatus(Enum): PUBLISHED = "Published" +def get_api_key_env_name(dandi_instance: DandiInstance) -> str: + """ + Get the name of the environment variable that can be used to specify the + API key for the given DANDI instance. + + :param dandi_instance: the DANDI instance + :return: the name of the environment variable + """ + return f"{dandi_instance.name.upper().replace('-', '_')}_API_KEY" + + # Following class is loosely based on GirderClient, with authentication etc # being stripped. # TODO: add copyright/license info @@ -487,9 +498,9 @@ def dandi_authenticate(self) -> None: """ Acquire and set the authentication token/API key used by the `DandiAPIClient`. - If the :envvar:`f"{self._instance_id.replace('-', '_')}_API_KEY"` environment - variable is set, its value is used as the token. Otherwise, the token is looked - up in the user's keyring under the service + If the :envvar:`f"{self.dandi_instance.name.upper().replace('-', '_')}_API_KEY"` + environment variable is set, its value is used as the token. Otherwise, + the token is looked up in the user's keyring under the service ":samp:`dandi-api-{self.dandi_instance.name}`" [#auth]_ and username "``key``". If no token is found there, the user is prompted for the token, and, if it proves to be valid, it is stored in the user's keyring. @@ -498,7 +509,7 @@ def dandi_authenticate(self) -> None: "``dandi-api-dandi-sandbox``" for the sandbox server """ # Shortcut for advanced folks - env_var_name = f"{self._instance_id.replace('-', '_')}_API_KEY" + env_var_name = get_api_key_env_name(self.dandi_instance) api_key = os.environ.get(env_var_name, None) if api_key: lgr.debug(f"Using `{env_var_name}` environment variable as the API key") From 9a8930c32f47b7fe972bfa30df57bcbfc06f88a1 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Tue, 28 Oct 2025 13:18:16 -0700 Subject: [PATCH 04/11] test: add tests for `test_get_api_key_env_name()` --- dandi/tests/test_dandiapi.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py index d445ea90f..0695906eb 100644 --- a/dandi/tests/test_dandiapi.py +++ b/dandi/tests/test_dandiapi.py @@ -23,6 +23,7 @@ from ..consts import ( DRAFT, VERSION_REGEX, + DandiInstance, dandiset_identifier_regex, dandiset_metadata_file, ) @@ -32,6 +33,7 @@ RemoteBlobAsset, RemoteZarrAsset, Version, + get_api_key_env_name, ) from ..download import download from ..exceptions import NotFoundError, SchemaVersionError @@ -833,3 +835,19 @@ def test_asset_as_readable_open(new_dandiset: SampleDandiset, tmp_path: Path) -> assert fp.read() == b"This is test text.\n" finally: fp.close() + + +@pytest.mark.parametrize( + ("instance_name", "expected_env_var_name"), + [ + ("dandi", "DANDI_API_KEY"), + ("dandi-api-local-docker-tests", "DANDI_API_LOCAL_DOCKER_TESTS_API_KEY"), + ("dandi-sandbox", "DANDI_SANDBOX_API_KEY"), + ("ember-sandbox", "EMBER_SANDBOX_API_KEY"), + ], +) +def test_get_api_key_env_name(instance_name: str, expected_env_var_name: str) -> None: + dandi_instance = DandiInstance( + name=instance_name, gui="https://example.com", api="https://api.example.com" + ) + assert get_api_key_env_name(dandi_instance) == expected_env_var_name From 473a8fcd2a74fb947bf2607b7b2942f82c2f8b25 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Tue, 28 Oct 2025 14:37:24 -0700 Subject: [PATCH 05/11] test: update tests for use of variable env var name for api key --- dandi/cli/tests/test_service_scripts.py | 11 +- dandi/tests/fixtures.py | 5 +- dandi/tests/test_dandiapi.py | 12 +- dandi/tests/test_delete.py | 77 ++++++++--- dandi/tests/test_keyring.py | 6 +- dandi/tests/test_move.py | 172 +++++++++++++++++++----- 6 files changed, 223 insertions(+), 60 deletions(-) diff --git a/dandi/cli/tests/test_service_scripts.py b/dandi/cli/tests/test_service_scripts.py index e1f3a85d3..4684516b4 100644 --- a/dandi/cli/tests/test_service_scripts.py +++ b/dandi/cli/tests/test_service_scripts.py @@ -13,6 +13,7 @@ import pytest from dandi import __version__ +from dandi.dandiapi import get_api_key_env_name from dandi.tests.fixtures import SampleDandiset from ..cmd_service_scripts import service_scripts @@ -31,7 +32,10 @@ def test_reextract_metadata( asset_id = nwb_dandiset.dandiset.get_asset_by_path( "sub-mouse001/sub-mouse001.nwb" ).identifier - monkeypatch.setenv("DANDI_API_KEY", nwb_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(nwb_dandiset.api.client.dandi_instance), + nwb_dandiset.api.api_key, + ) r = CliRunner().invoke( service_scripts, ["reextract-metadata", "--when=always", nwb_dandiset.dandiset.version_api_url], @@ -74,7 +78,10 @@ def test_update_dandiset_from_doi( ) -> None: dandiset_id = new_dandiset.dandiset_id repository = new_dandiset.api.instance.gui - monkeypatch.setenv("DANDI_API_KEY", new_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(new_dandiset.api.client.dandi_instance), + new_dandiset.api.api_key, + ) if os.environ.get("DANDI_TESTS_NO_VCR", "") or sys.version_info <= (3, 10): # Older vcrpy has an issue with Python 3.9 and newer urllib2 >= 2 # But we require newer urllib2 for more correct operation, and diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 541da8ac2..66d343acf 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -36,7 +36,7 @@ known_instances, metadata_nwb_file_fields, ) -from ..dandiapi import DandiAPIClient, RemoteDandiset +from ..dandiapi import DandiAPIClient, RemoteDandiset, get_api_key_env_name from ..pynwb_utils import make_nwb_file from ..upload import upload @@ -558,7 +558,8 @@ def client(self) -> DandiAPIClient: def upload(self, paths: list[str | Path] | None = None, **kwargs: Any) -> None: with pytest.MonkeyPatch().context() as m: - m.setenv("DANDI_API_KEY", self.api.api_key) + env_var_name = get_api_key_env_name(self.api.client.dandi_instance) + m.setenv(f"{env_var_name}", self.api.api_key) # todo: to be modified upload( paths=paths or [self.dspath], dandi_instance=self.api.instance_id, diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py index 0695906eb..08d8446e0 100644 --- a/dandi/tests/test_dandiapi.py +++ b/dandi/tests/test_dandiapi.py @@ -140,7 +140,9 @@ def test_authenticate_bad_key_good_key_input( ) confirm_mock = mocker.patch("click.confirm", return_value=True) - monkeypatch.delenv("DANDI_API_KEY", raising=False) + monkeypatch.delenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False + ) client = DandiAPIClient(local_dandi_api.api_url) assert "Authorization" not in client.session.headers @@ -171,7 +173,9 @@ def test_authenticate_good_key_keyring( is_interactive_spy = mocker.spy(dandiapi, "is_interactive") confirm_spy = mocker.spy(click, "confirm") - monkeypatch.delenv("DANDI_API_KEY", raising=False) + monkeypatch.delenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False + ) client = DandiAPIClient(local_dandi_api.api_url) assert "Authorization" not in client.session.headers @@ -203,7 +207,9 @@ def test_authenticate_bad_key_keyring_good_key_input( ) confirm_mock = mocker.patch("click.confirm", return_value=True) - monkeypatch.delenv("DANDI_API_KEY", raising=False) + monkeypatch.delenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False + ) client = DandiAPIClient(local_dandi_api.api_url) assert "Authorization" not in client.session.headers diff --git a/dandi/tests/test_delete.py b/dandi/tests/test_delete.py index b3cf4b583..1af8408f2 100644 --- a/dandi/tests/test_delete.py +++ b/dandi/tests/test_delete.py @@ -7,7 +7,7 @@ from .fixtures import DandiAPI, SampleDandiset from ..consts import DRAFT, dandiset_metadata_file -from ..dandiapi import RESTFullAPIClient +from ..dandiapi import RESTFullAPIClient, get_api_key_env_name from ..delete import delete, is_same_url from ..download import download from ..exceptions import NotFoundError @@ -67,7 +67,10 @@ def test_delete_paths( remainder: list[Path], ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -92,7 +95,10 @@ def test_delete_path_confirm( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -113,7 +119,10 @@ def test_delete_path_pyout( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") delete(["subdir2/coconut.txt"], dandi_instance=instance, force=True) @@ -143,7 +152,10 @@ def test_delete_dandiset( paths: list[str], ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -166,7 +178,10 @@ def test_delete_dandiset_confirm( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -187,7 +202,10 @@ def test_delete_dandiset_mismatch( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id not_dandiset = str(int(dandiset_id) - 1).zfill(6) @@ -216,7 +234,10 @@ def test_delete_instance_mismatch( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -242,7 +263,10 @@ def test_delete_instance_mismatch( def test_delete_nonexistent_dandiset( local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv("DANDI_API_KEY", local_dandi_api.api_key) + monkeypatch.setenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), + local_dandi_api.api_key, + ) instance = local_dandi_api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") with pytest.raises(NotFoundError) as excinfo: @@ -259,7 +283,10 @@ def test_delete_nonexistent_dandiset( def test_delete_nonexistent_dandiset_skip_missing( local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv("DANDI_API_KEY", local_dandi_api.api_key) + monkeypatch.setenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), + local_dandi_api.api_key, + ) instance = local_dandi_api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") delete( @@ -277,7 +304,10 @@ def test_delete_nonexistent_asset( monkeypatch: pytest.MonkeyPatch, text_dandiset: SampleDandiset, ) -> None: - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -304,7 +334,10 @@ def test_delete_nonexistent_asset_skip_missing( text_dandiset: SampleDandiset, tmp_path: Path, ) -> None: - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -333,7 +366,10 @@ def test_delete_nonexistent_asset_folder( monkeypatch: pytest.MonkeyPatch, text_dandiset: SampleDandiset, ) -> None: - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -360,7 +396,10 @@ def test_delete_nonexistent_asset_folder_skip_missing( text_dandiset: SampleDandiset, tmp_path: Path, ) -> None: - monkeypatch.setenv("DANDI_API_KEY", text_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(text_dandiset.api.client.dandi_instance), + text_dandiset.api.api_key, + ) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -387,7 +426,10 @@ def test_delete_nonexistent_asset_folder_skip_missing( def test_delete_version( local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv("DANDI_API_KEY", local_dandi_api.api_key) + monkeypatch.setenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), + local_dandi_api.api_key, + ) instance = local_dandi_api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") with pytest.raises(NotImplementedError) as excinfo: @@ -430,7 +472,10 @@ def test_delete_zarr_path( tmp_path: Path, ) -> None: monkeypatch.chdir(zarr_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", zarr_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(zarr_dandiset.api.client.dandi_instance), + zarr_dandiset.api.api_key, + ) instance = zarr_dandiset.api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") delete(["sample.zarr"], dandi_instance=instance, devel_debug=True, force=True) diff --git a/dandi/tests/test_keyring.py b/dandi/tests/test_keyring.py index 84bae065d..19ad3f51e 100644 --- a/dandi/tests/test_keyring.py +++ b/dandi/tests/test_keyring.py @@ -11,7 +11,7 @@ from pytest_mock import MockerFixture from .fixtures import DandiAPI -from ..dandiapi import DandiAPIClient +from ..dandiapi import DandiAPIClient, get_api_key_env_name from ..keyring import keyring_lookup, keyringrc_file @@ -29,7 +29,9 @@ def ensure_keyring_backends() -> None: def test_dandi_authenticate_no_env_var( local_dandi_api: DandiAPI, monkeypatch: pytest.MonkeyPatch, mocker: MockerFixture ) -> None: - monkeypatch.delenv("DANDI_API_KEY", raising=False) + monkeypatch.delenv( + get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False + ) monkeypatch.setenv("PYTHON_KEYRING_BACKEND", "keyring.backends.null.Keyring") inputmock = mocker.patch( "dandi.dandiapi.input", return_value=local_dandi_api.api_key diff --git a/dandi/tests/test_move.py b/dandi/tests/test_move.py index 3f1eb5e0d..5b43cfd96 100644 --- a/dandi/tests/test_move.py +++ b/dandi/tests/test_move.py @@ -7,7 +7,7 @@ import pytest from .fixtures import SampleDandiset -from ..dandiapi import RemoteAsset +from ..dandiapi import RemoteAsset, get_api_key_env_name from ..exceptions import NotFoundError from ..move import AssetMismatchError, MoveExisting, MoveWorkOn, move @@ -172,7 +172,10 @@ def test_move( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( *srcs, dest=dest, @@ -194,7 +197,10 @@ def test_move_skip( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir4/foo.json", @@ -221,7 +227,10 @@ def test_move_error( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -248,7 +257,10 @@ def test_move_overwrite( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir4/foo.json", @@ -275,7 +287,10 @@ def test_move_no_srcs( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( dest="nowhere", @@ -291,7 +306,10 @@ def test_move_regex_multisrcs( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( r"\.txt", @@ -317,7 +335,10 @@ def test_move_multisrcs_file_dest( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -343,7 +364,10 @@ def test_move_folder_src_file_dest( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( "subdir1", @@ -365,7 +389,10 @@ def test_move_nonexistent_src( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(NotFoundError) as excinfo: move( "file.txt", @@ -391,7 +418,10 @@ def test_move_file_slash_src( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -417,7 +447,10 @@ def test_move_file_slash_dest( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -437,7 +470,10 @@ def test_move_regex_no_match( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( "no-match", @@ -458,7 +494,10 @@ def test_move_regex_collision( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( r"^\w+/foo\.json$", @@ -486,7 +525,10 @@ def test_move_regex_some_to_self( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( r"(.+[123])/([^.]+)\.(.+)", dest=r"\1/\2.dat", @@ -528,7 +570,10 @@ def test_move_from_subdir( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "../file.txt", "apple.txt", @@ -558,7 +603,10 @@ def test_move_in_subdir( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "apple.txt", dest="macintosh.txt", @@ -584,7 +632,10 @@ def test_move_from_subdir_abspaths( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(NotFoundError) as excinfo: move( "file.txt", @@ -610,7 +661,10 @@ def test_move_from_subdir_as_dot( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(ValueError) as excinfo: move( ".", @@ -633,7 +687,10 @@ def test_move_from_subdir_regex( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( r"\.txt", dest=".dat", @@ -661,7 +718,10 @@ def test_move_from_subdir_regex_no_changes( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( r"\.txt", dest=".txt", @@ -685,7 +745,10 @@ def test_move_dandiset_path( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(tmp_path) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir2/banana.txt", @@ -715,7 +778,10 @@ def test_move_dandiset_url( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(tmp_path) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir2/banana.txt", @@ -740,7 +806,10 @@ def test_move_work_on_auto( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir2/banana.txt", @@ -777,7 +846,10 @@ def test_move_local_delete_empty_dirs( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir4") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "../subdir1/apple.txt", "../subdir2/banana.txt", @@ -807,7 +879,10 @@ def test_move_both_src_path_not_in_local( (moving_dandiset.dspath / "subdir2" / "banana.txt").unlink() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(AssetMismatchError) as excinfo: move( "subdir2", @@ -832,7 +907,10 @@ def test_move_both_src_path_not_in_remote( (moving_dandiset.dspath / "subdir2" / "mango.txt").write_text("Mango\n") starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(AssetMismatchError) as excinfo: move( "subdir2", @@ -857,7 +935,10 @@ def test_move_both_dest_path_not_in_remote( (moving_dandiset.dspath / "subdir2" / "file.txt").write_text("This is a file.\n") starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(AssetMismatchError) as excinfo: move( "file.txt", @@ -884,7 +965,10 @@ def test_move_both_dest_path_not_in_local( (moving_dandiset.dspath / "subdir2" / "banana.txt").unlink() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(AssetMismatchError) as excinfo: move( "file.txt", @@ -913,7 +997,10 @@ def test_move_both_dest_mismatch( (moving_dandiset.dspath / "subdir1" / "apple.txt" / "seeds").write_text("12345\n") starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) with pytest.raises(AssetMismatchError) as excinfo: move( "file.txt", @@ -943,7 +1030,10 @@ def test_move_pyout( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir4/foo.json", @@ -975,7 +1065,10 @@ def test_move_pyout_dry_run( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", "subdir4/foo.json", @@ -1001,7 +1094,10 @@ def test_move_path_to_self( (moving_dandiset.dspath / "newdir").mkdir() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "apple.txt", dest="../subdir1", @@ -1029,7 +1125,10 @@ def test_move_remote_dest_is_local_dir_sans_slash( (moving_dandiset.dspath / "newdir").mkdir() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", dest="newdir", @@ -1048,7 +1147,10 @@ def test_move_both_dest_is_local_dir_sans_slash( (moving_dandiset.dspath / "newdir").mkdir() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv("DANDI_API_KEY", moving_dandiset.api.api_key) + monkeypatch.setenv( + get_api_key_env_name(moving_dandiset.api.client.dandi_instance), + moving_dandiset.api.api_key, + ) move( "file.txt", dest="newdir", From 73a6c213f78d28ecd6da425bac6357a5e89ad083 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Tue, 28 Oct 2025 16:01:45 -0700 Subject: [PATCH 06/11] chore: remove unneeded todo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- dandi/tests/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 66d343acf..5f13168c8 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -559,7 +559,7 @@ def client(self) -> DandiAPIClient: def upload(self, paths: list[str | Path] | None = None, **kwargs: Any) -> None: with pytest.MonkeyPatch().context() as m: env_var_name = get_api_key_env_name(self.api.client.dandi_instance) - m.setenv(f"{env_var_name}", self.api.api_key) # todo: to be modified + m.setenv(f"{env_var_name}", self.api.api_key) upload( paths=paths or [self.dspath], dandi_instance=self.api.instance_id, From 2cef48763dffe7c2fbbb37ca962a17061ac346b3 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Tue, 28 Oct 2025 16:08:37 -0700 Subject: [PATCH 07/11] refactor: remove unneeded f-string expression --- dandi/tests/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 5f13168c8..9678f6dc5 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -559,7 +559,7 @@ def client(self) -> DandiAPIClient: def upload(self, paths: list[str | Path] | None = None, **kwargs: Any) -> None: with pytest.MonkeyPatch().context() as m: env_var_name = get_api_key_env_name(self.api.client.dandi_instance) - m.setenv(f"{env_var_name}", self.api.api_key) + m.setenv(env_var_name, self.api.api_key) upload( paths=paths or [self.dspath], dandi_instance=self.api.instance_id, From d91ff66cf44bab86579e31eee4fb18d1c77c4b57 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Fri, 31 Oct 2025 17:18:19 -0700 Subject: [PATCH 08/11] rf: refactor logic of monkey patching API key env into `DandiAPI` --- dandi/cli/tests/test_service_scripts.py | 11 +- dandi/tests/fixtures.py | 22 ++- dandi/tests/test_dandiapi.py | 12 +- dandi/tests/test_delete.py | 77 +++-------- dandi/tests/test_keyring.py | 6 +- dandi/tests/test_move.py | 172 +++++------------------- 6 files changed, 78 insertions(+), 222 deletions(-) diff --git a/dandi/cli/tests/test_service_scripts.py b/dandi/cli/tests/test_service_scripts.py index 4684516b4..5cb2e9c38 100644 --- a/dandi/cli/tests/test_service_scripts.py +++ b/dandi/cli/tests/test_service_scripts.py @@ -13,7 +13,6 @@ import pytest from dandi import __version__ -from dandi.dandiapi import get_api_key_env_name from dandi.tests.fixtures import SampleDandiset from ..cmd_service_scripts import service_scripts @@ -32,10 +31,7 @@ def test_reextract_metadata( asset_id = nwb_dandiset.dandiset.get_asset_by_path( "sub-mouse001/sub-mouse001.nwb" ).identifier - monkeypatch.setenv( - get_api_key_env_name(nwb_dandiset.api.client.dandi_instance), - nwb_dandiset.api.api_key, - ) + nwb_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) r = CliRunner().invoke( service_scripts, ["reextract-metadata", "--when=always", nwb_dandiset.dandiset.version_api_url], @@ -78,10 +74,7 @@ def test_update_dandiset_from_doi( ) -> None: dandiset_id = new_dandiset.dandiset_id repository = new_dandiset.api.instance.gui - monkeypatch.setenv( - get_api_key_env_name(new_dandiset.api.client.dandi_instance), - new_dandiset.api.api_key, - ) + new_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) if os.environ.get("DANDI_TESTS_NO_VCR", "") or sys.version_info <= (3, 10): # Older vcrpy has an issue with Python 3.9 and newer urllib2 >= 2 # But we require newer urllib2 for more correct operation, and diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index 9678f6dc5..cf985fc84 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -535,6 +535,25 @@ def instance_id(self) -> str: def api_url(self) -> str: return self.instance.api + def monkeypatch_set_api_key_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + Monkeypatch the environment variable that provides the API key for accessing + the associated DANDI instance + """ + monkeypatch.setenv( + get_api_key_env_name(self.client.dandi_instance), + self.api_key, + ) + + def monkeypatch_del_api_key_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + Monkeypatch to remove the environment variable that provides the API key for + accessing the associated DANDI instance. + """ + monkeypatch.delenv( + get_api_key_env_name(self.client.dandi_instance), raising=False + ) + @pytest.fixture(scope="session") def local_dandi_api(docker_compose_setup: dict[str, str]) -> Iterator[DandiAPI]: @@ -558,8 +577,7 @@ def client(self) -> DandiAPIClient: def upload(self, paths: list[str | Path] | None = None, **kwargs: Any) -> None: with pytest.MonkeyPatch().context() as m: - env_var_name = get_api_key_env_name(self.api.client.dandi_instance) - m.setenv(env_var_name, self.api.api_key) + self.api.monkeypatch_set_api_key_env(m) upload( paths=paths or [self.dspath], dandi_instance=self.api.instance_id, diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py index 08d8446e0..e0195c694 100644 --- a/dandi/tests/test_dandiapi.py +++ b/dandi/tests/test_dandiapi.py @@ -140,9 +140,7 @@ def test_authenticate_bad_key_good_key_input( ) confirm_mock = mocker.patch("click.confirm", return_value=True) - monkeypatch.delenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False - ) + local_dandi_api.monkeypatch_del_api_key_env(monkeypatch) client = DandiAPIClient(local_dandi_api.api_url) assert "Authorization" not in client.session.headers @@ -173,9 +171,7 @@ def test_authenticate_good_key_keyring( is_interactive_spy = mocker.spy(dandiapi, "is_interactive") confirm_spy = mocker.spy(click, "confirm") - monkeypatch.delenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False - ) + local_dandi_api.monkeypatch_del_api_key_env(monkeypatch) client = DandiAPIClient(local_dandi_api.api_url) assert "Authorization" not in client.session.headers @@ -207,9 +203,7 @@ def test_authenticate_bad_key_keyring_good_key_input( ) confirm_mock = mocker.patch("click.confirm", return_value=True) - monkeypatch.delenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False - ) + local_dandi_api.monkeypatch_del_api_key_env(monkeypatch) client = DandiAPIClient(local_dandi_api.api_url) assert "Authorization" not in client.session.headers diff --git a/dandi/tests/test_delete.py b/dandi/tests/test_delete.py index 1af8408f2..0bd0d2b9d 100644 --- a/dandi/tests/test_delete.py +++ b/dandi/tests/test_delete.py @@ -7,7 +7,7 @@ from .fixtures import DandiAPI, SampleDandiset from ..consts import DRAFT, dandiset_metadata_file -from ..dandiapi import RESTFullAPIClient, get_api_key_env_name +from ..dandiapi import RESTFullAPIClient from ..delete import delete, is_same_url from ..download import download from ..exceptions import NotFoundError @@ -67,10 +67,7 @@ def test_delete_paths( remainder: list[Path], ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -95,10 +92,7 @@ def test_delete_path_confirm( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -119,10 +113,7 @@ def test_delete_path_pyout( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") delete(["subdir2/coconut.txt"], dandi_instance=instance, force=True) @@ -152,10 +143,7 @@ def test_delete_dandiset( paths: list[str], ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -178,10 +166,7 @@ def test_delete_dandiset_confirm( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -202,10 +187,7 @@ def test_delete_dandiset_mismatch( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id not_dandiset = str(int(dandiset_id) - 1).zfill(6) @@ -234,10 +216,7 @@ def test_delete_instance_mismatch( text_dandiset: SampleDandiset, ) -> None: monkeypatch.chdir(text_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -263,10 +242,7 @@ def test_delete_instance_mismatch( def test_delete_nonexistent_dandiset( local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), - local_dandi_api.api_key, - ) + local_dandi_api.monkeypatch_set_api_key_env(monkeypatch) instance = local_dandi_api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") with pytest.raises(NotFoundError) as excinfo: @@ -283,10 +259,7 @@ def test_delete_nonexistent_dandiset( def test_delete_nonexistent_dandiset_skip_missing( local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), - local_dandi_api.api_key, - ) + local_dandi_api.monkeypatch_set_api_key_env(monkeypatch) instance = local_dandi_api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") delete( @@ -304,10 +277,7 @@ def test_delete_nonexistent_asset( monkeypatch: pytest.MonkeyPatch, text_dandiset: SampleDandiset, ) -> None: - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -334,10 +304,7 @@ def test_delete_nonexistent_asset_skip_missing( text_dandiset: SampleDandiset, tmp_path: Path, ) -> None: - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -366,10 +333,7 @@ def test_delete_nonexistent_asset_folder( monkeypatch: pytest.MonkeyPatch, text_dandiset: SampleDandiset, ) -> None: - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -396,10 +360,7 @@ def test_delete_nonexistent_asset_folder_skip_missing( text_dandiset: SampleDandiset, tmp_path: Path, ) -> None: - monkeypatch.setenv( - get_api_key_env_name(text_dandiset.api.client.dandi_instance), - text_dandiset.api.api_key, - ) + text_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = text_dandiset.api.instance_id dandiset_id = text_dandiset.dandiset_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") @@ -426,10 +387,7 @@ def test_delete_nonexistent_asset_folder_skip_missing( def test_delete_version( local_dandi_api: DandiAPI, mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), - local_dandi_api.api_key, - ) + local_dandi_api.monkeypatch_set_api_key_env(monkeypatch) instance = local_dandi_api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") with pytest.raises(NotImplementedError) as excinfo: @@ -472,10 +430,7 @@ def test_delete_zarr_path( tmp_path: Path, ) -> None: monkeypatch.chdir(zarr_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(zarr_dandiset.api.client.dandi_instance), - zarr_dandiset.api.api_key, - ) + zarr_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) instance = zarr_dandiset.api.instance_id delete_spy = mocker.spy(RESTFullAPIClient, "delete") delete(["sample.zarr"], dandi_instance=instance, devel_debug=True, force=True) diff --git a/dandi/tests/test_keyring.py b/dandi/tests/test_keyring.py index 19ad3f51e..d4bf82667 100644 --- a/dandi/tests/test_keyring.py +++ b/dandi/tests/test_keyring.py @@ -11,7 +11,7 @@ from pytest_mock import MockerFixture from .fixtures import DandiAPI -from ..dandiapi import DandiAPIClient, get_api_key_env_name +from ..dandiapi import DandiAPIClient from ..keyring import keyring_lookup, keyringrc_file @@ -29,9 +29,7 @@ def ensure_keyring_backends() -> None: def test_dandi_authenticate_no_env_var( local_dandi_api: DandiAPI, monkeypatch: pytest.MonkeyPatch, mocker: MockerFixture ) -> None: - monkeypatch.delenv( - get_api_key_env_name(local_dandi_api.client.dandi_instance), raising=False - ) + local_dandi_api.monkeypatch_del_api_key_env(monkeypatch) monkeypatch.setenv("PYTHON_KEYRING_BACKEND", "keyring.backends.null.Keyring") inputmock = mocker.patch( "dandi.dandiapi.input", return_value=local_dandi_api.api_key diff --git a/dandi/tests/test_move.py b/dandi/tests/test_move.py index 5b43cfd96..625bef307 100644 --- a/dandi/tests/test_move.py +++ b/dandi/tests/test_move.py @@ -7,7 +7,7 @@ import pytest from .fixtures import SampleDandiset -from ..dandiapi import RemoteAsset, get_api_key_env_name +from ..dandiapi import RemoteAsset from ..exceptions import NotFoundError from ..move import AssetMismatchError, MoveExisting, MoveWorkOn, move @@ -172,10 +172,7 @@ def test_move( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( *srcs, dest=dest, @@ -197,10 +194,7 @@ def test_move_skip( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir4/foo.json", @@ -227,10 +221,7 @@ def test_move_error( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -257,10 +248,7 @@ def test_move_overwrite( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir4/foo.json", @@ -287,10 +275,7 @@ def test_move_no_srcs( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( dest="nowhere", @@ -306,10 +291,7 @@ def test_move_regex_multisrcs( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( r"\.txt", @@ -335,10 +317,7 @@ def test_move_multisrcs_file_dest( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -364,10 +343,7 @@ def test_move_folder_src_file_dest( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( "subdir1", @@ -389,10 +365,7 @@ def test_move_nonexistent_src( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(NotFoundError) as excinfo: move( "file.txt", @@ -418,10 +391,7 @@ def test_move_file_slash_src( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -447,10 +417,7 @@ def test_move_file_slash_dest( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( "file.txt", @@ -470,10 +437,7 @@ def test_move_regex_no_match( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( "no-match", @@ -494,10 +458,7 @@ def test_move_regex_collision( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( r"^\w+/foo\.json$", @@ -525,10 +486,7 @@ def test_move_regex_some_to_self( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( r"(.+[123])/([^.]+)\.(.+)", dest=r"\1/\2.dat", @@ -570,10 +528,7 @@ def test_move_from_subdir( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "../file.txt", "apple.txt", @@ -603,10 +558,7 @@ def test_move_in_subdir( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "apple.txt", dest="macintosh.txt", @@ -632,10 +584,7 @@ def test_move_from_subdir_abspaths( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(NotFoundError) as excinfo: move( "file.txt", @@ -661,10 +610,7 @@ def test_move_from_subdir_as_dot( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(ValueError) as excinfo: move( ".", @@ -687,10 +633,7 @@ def test_move_from_subdir_regex( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( r"\.txt", dest=".dat", @@ -718,10 +661,7 @@ def test_move_from_subdir_regex_no_changes( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( r"\.txt", dest=".txt", @@ -745,10 +685,7 @@ def test_move_dandiset_path( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(tmp_path) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir2/banana.txt", @@ -778,10 +715,7 @@ def test_move_dandiset_url( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(tmp_path) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir2/banana.txt", @@ -806,10 +740,7 @@ def test_move_work_on_auto( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir2/banana.txt", @@ -846,10 +777,7 @@ def test_move_local_delete_empty_dirs( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir4") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "../subdir1/apple.txt", "../subdir2/banana.txt", @@ -879,10 +807,7 @@ def test_move_both_src_path_not_in_local( (moving_dandiset.dspath / "subdir2" / "banana.txt").unlink() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(AssetMismatchError) as excinfo: move( "subdir2", @@ -907,10 +832,7 @@ def test_move_both_src_path_not_in_remote( (moving_dandiset.dspath / "subdir2" / "mango.txt").write_text("Mango\n") starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(AssetMismatchError) as excinfo: move( "subdir2", @@ -935,10 +857,7 @@ def test_move_both_dest_path_not_in_remote( (moving_dandiset.dspath / "subdir2" / "file.txt").write_text("This is a file.\n") starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(AssetMismatchError) as excinfo: move( "file.txt", @@ -965,10 +884,7 @@ def test_move_both_dest_path_not_in_local( (moving_dandiset.dspath / "subdir2" / "banana.txt").unlink() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(AssetMismatchError) as excinfo: move( "file.txt", @@ -997,10 +913,7 @@ def test_move_both_dest_mismatch( (moving_dandiset.dspath / "subdir1" / "apple.txt" / "seeds").write_text("12345\n") starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) with pytest.raises(AssetMismatchError) as excinfo: move( "file.txt", @@ -1030,10 +943,7 @@ def test_move_pyout( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir4/foo.json", @@ -1065,10 +975,7 @@ def test_move_pyout_dry_run( ) -> None: starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", "subdir4/foo.json", @@ -1094,10 +1001,7 @@ def test_move_path_to_self( (moving_dandiset.dspath / "newdir").mkdir() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath / "subdir1") - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "apple.txt", dest="../subdir1", @@ -1125,10 +1029,7 @@ def test_move_remote_dest_is_local_dir_sans_slash( (moving_dandiset.dspath / "newdir").mkdir() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", dest="newdir", @@ -1147,10 +1048,7 @@ def test_move_both_dest_is_local_dir_sans_slash( (moving_dandiset.dspath / "newdir").mkdir() starting_assets = list(moving_dandiset.dandiset.get_assets()) monkeypatch.chdir(moving_dandiset.dspath) - monkeypatch.setenv( - get_api_key_env_name(moving_dandiset.api.client.dandi_instance), - moving_dandiset.api.api_key, - ) + moving_dandiset.api.monkeypatch_set_api_key_env(monkeypatch) move( "file.txt", dest="newdir", From 84be13a3251237f07dbbc70b296c8986ccd3b259 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Fri, 31 Oct 2025 23:52:09 -0700 Subject: [PATCH 09/11] rf: refactor logic of generating API key env name into `DandiAPIClient` property --- dandi/dandiapi.py | 21 +++++++++------------ dandi/tests/fixtures.py | 8 +++----- dandi/tests/test_dandiapi.py | 11 ++++++----- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/dandi/dandiapi.py b/dandi/dandiapi.py index 86c00b57c..2550ae7e2 100644 --- a/dandi/dandiapi.py +++ b/dandi/dandiapi.py @@ -77,17 +77,6 @@ class VersionStatus(Enum): PUBLISHED = "Published" -def get_api_key_env_name(dandi_instance: DandiInstance) -> str: - """ - Get the name of the environment variable that can be used to specify the - API key for the given DANDI instance. - - :param dandi_instance: the DANDI instance - :return: the name of the environment variable - """ - return f"{dandi_instance.name.upper().replace('-', '_')}_API_KEY" - - # Following class is loosely based on GirderClient, with authentication etc # being stripped. # TODO: add copyright/license info @@ -509,7 +498,7 @@ def dandi_authenticate(self) -> None: "``dandi-api-dandi-sandbox``" for the sandbox server """ # Shortcut for advanced folks - env_var_name = get_api_key_env_name(self.dandi_instance) + env_var_name = self.api_key_env_var api_key = os.environ.get(env_var_name, None) if api_key: lgr.debug(f"Using `{env_var_name}` environment variable as the API key") @@ -698,6 +687,14 @@ def get_asset(self, asset_id: str) -> BaseRemoteAsset: metadata = info.pop("metadata", None) return BaseRemoteAsset.from_base_data(self, info, metadata) + @property + def api_key_env_var(self) -> str: + """ + Get the name of the environment variable that can be used to specify the + API key for the associated DANDI instance. + """ + return f"{self.dandi_instance.name.upper().replace('-', '_')}_API_KEY" + # `arbitrary_types_allowed` is needed for `client: DandiAPIClient` class APIBase(BaseModel, populate_by_name=True, arbitrary_types_allowed=True): diff --git a/dandi/tests/fixtures.py b/dandi/tests/fixtures.py index cf985fc84..d616d2871 100644 --- a/dandi/tests/fixtures.py +++ b/dandi/tests/fixtures.py @@ -36,7 +36,7 @@ known_instances, metadata_nwb_file_fields, ) -from ..dandiapi import DandiAPIClient, RemoteDandiset, get_api_key_env_name +from ..dandiapi import DandiAPIClient, RemoteDandiset from ..pynwb_utils import make_nwb_file from ..upload import upload @@ -541,7 +541,7 @@ def monkeypatch_set_api_key_env(self, monkeypatch: pytest.MonkeyPatch) -> None: the associated DANDI instance """ monkeypatch.setenv( - get_api_key_env_name(self.client.dandi_instance), + self.client.api_key_env_var, self.api_key, ) @@ -550,9 +550,7 @@ def monkeypatch_del_api_key_env(self, monkeypatch: pytest.MonkeyPatch) -> None: Monkeypatch to remove the environment variable that provides the API key for accessing the associated DANDI instance. """ - monkeypatch.delenv( - get_api_key_env_name(self.client.dandi_instance), raising=False - ) + monkeypatch.delenv(self.client.api_key_env_var, raising=False) @pytest.fixture(scope="session") diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py index e0195c694..b92489ad2 100644 --- a/dandi/tests/test_dandiapi.py +++ b/dandi/tests/test_dandiapi.py @@ -33,7 +33,6 @@ RemoteBlobAsset, RemoteZarrAsset, Version, - get_api_key_env_name, ) from ..download import download from ..exceptions import NotFoundError, SchemaVersionError @@ -846,8 +845,10 @@ def test_asset_as_readable_open(new_dandiset: SampleDandiset, tmp_path: Path) -> ("ember-sandbox", "EMBER_SANDBOX_API_KEY"), ], ) -def test_get_api_key_env_name(instance_name: str, expected_env_var_name: str) -> None: - dandi_instance = DandiInstance( - name=instance_name, gui="https://example.com", api="https://api.example.com" +def test_get_api_key_env_var(instance_name: str, expected_env_var_name: str) -> None: + dandi_api_client = DandiAPIClient( + dandi_instance=DandiInstance( + name=instance_name, gui="https://example.com", api="https://api.example.com" + ) ) - assert get_api_key_env_name(dandi_instance) == expected_env_var_name + assert dandi_api_client.api_key_env_var == expected_env_var_name From 24a6d3f6e99f4ede7ff1d37c07acb5e168c2cf22 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Sat, 1 Nov 2025 00:06:57 -0700 Subject: [PATCH 10/11] docs: correct the docstring for `dandi_authenticate` Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- dandi/dandiapi.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dandi/dandiapi.py b/dandi/dandiapi.py index 2550ae7e2..303bada94 100644 --- a/dandi/dandiapi.py +++ b/dandi/dandiapi.py @@ -487,12 +487,13 @@ def dandi_authenticate(self) -> None: """ Acquire and set the authentication token/API key used by the `DandiAPIClient`. - If the :envvar:`f"{self.dandi_instance.name.upper().replace('-', '_')}_API_KEY"` - environment variable is set, its value is used as the token. Otherwise, - the token is looked up in the user's keyring under the service - ":samp:`dandi-api-{self.dandi_instance.name}`" [#auth]_ and username "``key``". - If no token is found there, the user is prompted for the token, and, if - it proves to be valid, it is stored in the user's keyring. + If the :envvar:`{INSTANCE_NAME}_API_KEY` environment variable is set, its value + is used as the token. Here, ``{INSTANCE_NAME}`` is the uppercased instance name + with hyphens replaced by underscores. Otherwise, the token is looked up in the + user's keyring under the service ":samp:`dandi-api-{self.dandi_instance.name}`" + [#auth]_ and username "``key``". If no token is found there, the user is + prompted for the token, and, if it proves to be valid, it is stored in the + user's keyring. .. [#auth] E.g., "``dandi-api-dandi``" for the production server or "``dandi-api-dandi-sandbox``" for the sandbox server From 73694d0c8b8c5c0945d828fcc78cb3e6028b4245 Mon Sep 17 00:00:00 2001 From: Isaac To Date: Sat, 1 Nov 2025 00:08:25 -0700 Subject: [PATCH 11/11] docs: correct grammatical error in `DEVELOPMENT.md` Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- DEVELOPMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 624b841cb..cbaf7beab 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -63,7 +63,7 @@ development command line options. for the known instance named `dandi` and the environment variable `EMBER_SANDBOX_API_KEY` provides the key for the known instance named `ember-sandbox`. I.e., the environment variable name is the capitalized version of the instance's name - with "-" replaced by "_" suffixed by "_API_KEY". Providing API through environment + with "-" replaced by "_" suffixed by "_API_KEY". Providing API keys through environment variables avoids using keyrings, thus making it possible to "temporarily" use another account etc for the "API" version of the server.