Skip to content

Conversation

@candleindark
Copy link
Member

@candleindark candleindark commented Jul 9, 2025

This PR brings new features to allow the DANDI client to vendorize itself according to a DANDI sever instance it interacts with. It closes #1660.

TODOs:

  • Fetch server instance config JSON from a given DANDI server instance
  • Reconstruct server instance config object from the server with validation
  • [x] Set the instance config in the CLI initialization process
  • [ ] Implement way to save the DANDI server instance choice across different executions of commands
  • [ ] Implement a command to allow user to set server instance choice
  • Ensure dandishcema.models is not loaded at the start of each command that can take an instance option
    • This involves delaying import of dandishcema.models in numerous modules
  • Make sure the dandischema dependency is not longer pointing to Vendor-Configurable Metadata Models dandi-schema#294

Release Notes

  1. DUMMY_DANDI_ETAG and DUMMY_DANDI_ZARR_CHECKSUM in dandi/misctypes.py are replaced with get_dummy_dandi_etag() and get_dummy_dandi_zarr_checksum in order to delay the import of dandischema.models.

…e` branch

This commit should be dropped before the
containing PR is merged. This is a temporary
measure to test the behavior of `dandischema`
with the changes in
dandi/dandi-schema#294
@codecov
Copy link

codecov bot commented Jul 9, 2025

Codecov Report

Attention: Patch coverage is 74.02597% with 20 lines in your changes missing coverage. Please review.

Project coverage is 66.75%. Comparing base (03b7c07) to head (066e16e).

Files with missing lines Patch % Lines
dandi/cli/base.py 50.00% 8 Missing ⚠️
dandi/files/bases.py 64.28% 5 Missing ⚠️
dandi/cli/cmd_ls.py 0.00% 1 Missing ⚠️
dandi/dandiset.py 0.00% 1 Missing ⚠️
dandi/files/bids.py 66.66% 1 Missing ⚠️
dandi/files/zarr.py 85.71% 1 Missing ⚠️
dandi/metadata/nwb.py 50.00% 1 Missing ⚠️
dandi/misctypes.py 91.66% 1 Missing ⚠️
dandi/tests/test_metadata.py 50.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (03b7c07) and HEAD (066e16e). Click for more details.

HEAD has 552 uploads less than BASE
Flag BASE (03b7c07) HEAD (066e16e)
unittests 553 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1661       +/-   ##
===========================================
- Coverage   88.84%   66.75%   -22.10%     
===========================================
  Files          83       83               
  Lines       11515    11537       +22     
===========================================
- Hits        10230     7701     -2529     
- Misses       1285     3836     +2551     
Flag Coverage Δ
unittests 66.75% <74.02%> (-22.10%) ⬇️

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.

@candleindark candleindark changed the title Vendorization dandi-cli per config in server info Vendorize dandi-cli per config in server info Jul 10, 2025
This function binds the DANDI client
to a specific DANDI server instance so
that subsequence commands executed
by the client is executed in the context
of the DANDI sever instance
This function should be run before a
command is executed so that it can
set the context of the execution such
as which DANDI sever instance to interact
with.
from dandischema.models import Dandiset as DandisetMeta

# noinspection PyUnresolvedReferences
from dandi.misctypes import BasePath, Digest, LocalReadableFile

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'BasePath' is not used.
@candleindark
Copy link
Member Author

@yarikoptic I have successfully delayed the import of dandischema.models in dandi-cli such that each command starts with dandischema.models not yet imported. Now I can insert code at the start of each command to set the instance config defined in dandischema.conf before a possible import of dandischema.models in the execution of the command. As you can see, the code relies on the ID of a known DANDI instance.

The commands in the following files have the instance_option, so what instance to use to set the instance config is clear, with the exception of cmd_download.py which involves some indirect logic.

  1. dandi/cli/cmd_delete.py
  2. dandi/cli/cmd_download.py
  3. dandi/cli/cmd_move.py
  4. dandi/cli/cmd_service_scripts.py
  5. dandi/cli/cmd_upload.py

On the other hand, the commands in the following files lack the instance_option, and what instance to use to set the instance config is unclear, or undetermined.

  1. dandi/cli/cmd_digest.py
  2. dandi/cli/cmd_instances.py
  3. dandi/cli/cmd_ls.py
  4. dandi/cli/cmd_organize.py
  5. dandi/cli/cmd_shell_completion.py
  6. dandi/cli/cmd_validate.py

Not all commands in these files need to import dandischema.models, but dandi/cli/cmd_ls.py, dandi/cli/cmd_organize.py, and dandi/cli/cmd_validate.py most likely do. We need to decide which DANDI instance to use to set the instance config in those that do.

In coming up with that decision, we may want to consider changing the interface and/or design of this client. We may want to have a centralized command that configures the client to be associated with a DANDI instance, and use the instance to set the instance config in subsequent command executions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Allow vendorization according to config in server info

1 participant