Obtain API key for DANDI instance from corresponding env var by candleindark · Pull Request #1731 · dandi/dandi-cli

candleindark · 2025-10-27T20:52:28Z

This PR closes #1730

Release Notes

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".

codecov · 2025-10-27T20:55:51Z

Codecov Report

❌ Patch coverage is 94.28571% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.07%. Comparing base (a79fffc) to head (73694d0).
⚠️ Report is 92 commits behind head on master.

Files with missing lines	Patch %	Lines
dandi/dandiapi.py	66.66%	2 Missing ⚠️
dandi/tests/fixtures.py	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1731      +/-   ##
==========================================
- Coverage   75.11%   75.07%   -0.05%     
==========================================
  Files          84       84              
  Lines       11856    11868      +12     
==========================================
+ Hits         8906     8910       +4     
- Misses       2950     2958       +8

Flag	Coverage Δ
unittests	`75.07% <94.28%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

yarikoptic · 2025-10-27T23:12:06Z

@dandi/ember WDYT about this? do your docs ATM instruct to use DANDI_API_KEY already?

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

Copilot

Pull Request Overview

This PR refactors the handling of API keys for DANDI instances from using a single environment variable DANDI_API_KEY to instance-specific environment variables. This change allows multiple DANDI instances to have their own API keys set simultaneously through environment variables.

Key Changes:

Introduced get_api_key_env_name() function to generate instance-specific environment variable names
Updated all test files to use the new instance-specific environment variable names
Updated documentation to reflect the new environment variable naming convention

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
dandi/dandiapi.py	Added `get_api_key_env_name()` function and updated authentication logic to use instance-specific environment variables
dandi/tests/test_move.py	Updated all tests to use instance-specific API key environment variables via `get_api_key_env_name()`
dandi/tests/test_keyring.py	Updated test to delete instance-specific API key environment variable
dandi/tests/test_delete.py	Updated all tests to use instance-specific API key environment variables
dandi/tests/test_dandiapi.py	Added test for `get_api_key_env_name()` function and updated authentication tests
dandi/tests/fixtures.py	Updated `SampleDandiset.upload()` to use instance-specific environment variables
dandi/cli/tests/test_service_scripts.py	Updated tests to use instance-specific environment variables
DEVELOPMENT.md	Updated documentation to describe the new environment variable naming scheme

Comments suppressed due to low confidence (1)

DEVELOPMENT.md:1

Corrected spelling of 'DAND_API_KEY' to 'DANDI_API_KEY'.

# DANDI Client Development

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dandi/tests/fixtures.py

dandi/dandiapi.py

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DEVELOPMENT.md

dandi/cli/tests/test_service_scripts.py

…ndiInstance`

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

yarikoptic

"compress" code a bit to better centralize and reduce code clutter

yarikoptic · 2025-10-29T16:06:40Z

dandi/dandiapi.py

+    :param dandi_instance: the DANDI instance
+    :return: the name of the environment variable
+    """
+    return f"{dandi_instance.name.upper().replace('-', '_')}_API_KEY"


looking at multitude of calls , I wonder if we should add api_key_envvar property to dandi_instance's class and be done to use it -- that would make code shorter.

Practically, what you proposed is more convenient. Conceptually, it is a bit awkward since the API key environment variable name is not really a property or attribute of a DANDI instance.

here -- we agreed for it to be moved as a _get_api_key_env_var method of DandiAPIClient

This logic is refactored into the api_key_env_var property of DandiAPIClient instead since it is actually used externally, https://github.com/candleindark/dandi-cli/blob/84be13a3251237f07dbbc70b296c8986ccd3b259/dandi/tests/fixtures.py#L544.

dandi/tests/test_move.py

NEStock · 2025-10-30T13:20:50Z

@dandi/ember WDYT about this? do your docs ATM instruct to use DANDI_API_KEY already?

@yarikoptic
I thought we did but I just did a quick check and I don't see any references to the term DANDI_API_KEY publically

…t` property

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dandi/dandiapi.py

DEVELOPMENT.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-actions · 2025-11-15T02:57:21Z

🚀 PR was released in 0.73.2 🚀

dandi-cli 0.73.2 (released Nov 15, 2025) changed API key lookup from the generic DANDI_API_KEY env var to instance-specific env vars (e.g. DANDI_SANDBOX_API_KEY for the sandbox). This broke sandbox uploads because nwb-guide only set DANDI_API_KEY. Now set both DANDI_API_KEY and DANDI_SANDBOX_API_KEY when uploading to the sandbox, maintaining compatibility with both old and new dandi. See: dandi/dandi-cli#1731 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

candleindark force-pushed the no-hardcoded-api-key-name branch from 3040cdd to 01d34a2 Compare October 28, 2025 18:32

candleindark added the patch Increment the patch version when merged label Oct 28, 2025

github-advanced-security bot found potential problems Oct 28, 2025

View reviewed changes

dandi/dandiapi.py Dismissed Show dismissed Hide dismissed

candleindark marked this pull request as ready for review October 28, 2025 22:55

candleindark requested a review from Copilot October 28, 2025 22:56

Copilot AI reviewed Oct 28, 2025

View reviewed changes

dandi/tests/fixtures.py Outdated Show resolved Hide resolved

dandi/tests/fixtures.py Outdated Show resolved Hide resolved

dandi/dandiapi.py Outdated Show resolved Hide resolved

candleindark requested review from Copilot and yarikoptic October 28, 2025 23:04

Copilot AI reviewed Oct 28, 2025

View reviewed changes

DEVELOPMENT.md Outdated Show resolved Hide resolved

dandi/cli/tests/test_service_scripts.py Outdated Show resolved Hide resolved

candleindark and others added 6 commits October 28, 2025 22:23

docs: adjust documentation

8170d36

feat: def utility function to get env var name for API key from a `Da…

5a21bd6

…ndiInstance`

test: add tests for test_get_api_key_env_name()

9a8930c

test: update tests for use of variable env var name for api key

473a8fc

chore: remove unneeded todo

73a6c21

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

refactor: remove unneeded f-string expression

2cef487

candleindark force-pushed the no-hardcoded-api-key-name branch from a024555 to 2cef487 Compare October 29, 2025 05:24

yarikoptic requested changes Oct 29, 2025

View reviewed changes

candleindark added 2 commits October 31, 2025 17:18

rf: refactor logic of monkey patching API key env into DandiAPI

d91ff66

rf: refactor logic of generating API key env name into `DandiAPIClien…

84be13a

…t` property

candleindark requested a review from Copilot November 1, 2025 06:59

Copilot AI reviewed Nov 1, 2025

View reviewed changes

dandi/dandiapi.py Outdated Show resolved Hide resolved

DEVELOPMENT.md Outdated Show resolved Hide resolved

candleindark and others added 2 commits November 1, 2025 00:12

docs: correct the docstring for dandi_authenticate

24a6d3f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

docs: correct grammatical error in DEVELOPMENT.md

73694d0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

candleindark force-pushed the no-hardcoded-api-key-name branch from f8c29cd to 73694d0 Compare November 1, 2025 07:13

candleindark requested a review from yarikoptic November 1, 2025 21:58

yarikoptic approved these changes Nov 3, 2025

View reviewed changes

yarikoptic merged commit dad37b8 into dandi:master Nov 3, 2025
40 checks passed

candleindark deleted the no-hardcoded-api-key-name branch November 6, 2025 17:01

github-actions bot added the released label Nov 15, 2025

candleindark mentioned this pull request Dec 2, 2025

Fix submodule URL updates to only modify installed submodules dandi/backups2datalad#96

Merged

bendichter mentioned this pull request Feb 11, 2026

Fix DANDI sandbox upload broken by dandi-cli 0.73.2 NeurodataWithoutBorders/nwb-guide#1037

Merged

2 tasks

Conversation

candleindark commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

Uh oh!

codecov bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yarikoptic commented Oct 27, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

yarikoptic left a comment

Choose a reason for hiding this comment

Uh oh!

yarikoptic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

candleindark Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

yarikoptic Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

candleindark Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NEStock commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

candleindark commented Oct 27, 2025 •

edited

Loading

codecov bot commented Oct 27, 2025 •

edited

Loading

candleindark Nov 1, 2025 •

edited

Loading

NEStock commented Oct 30, 2025 •

edited

Loading