Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 21, 2025

  • Modify regex in dandi/dandiarchive.py to dynamically build the instance name pattern from known_instances
  • Update display string from DANDI:<dandiset id>[/<version>] to <INSTANCE>:<dandiset id>[/<version>]
  • Add tests for DANDI-SANDBOX, LINC, EMBER, and ember-sandbox instances
  • Update documentation in docs/source/ref/urls.rst to reflect all known instances support
  • Verify tests pass and code review has no issues
Original prompt

This section details on the original issue you should resolve

<issue_title>Adjust known_urls to support known instances</issue_title>
<issue_description>ATM we know many instances already but known_urls has it hardcoded for DANDI: only so download from any other would fail, e.g.

❯ dandi download DANDI-SANDBOX:000029
2025-11-21 15:27:26,566 [    INFO] Logs saved in /home/yoh/.local/state/dandi-cli/log/2025.11.21-20.27.26Z-117205.log
Error: We do not know how to map URL DANDI-SANDBOX:000029 to our servers.
Accepted resource identifier patterns:
 - DANDI:<dandiset id>[/<version>]
 - https://gui.dandiarchive.org/...
 - https://identifiers.org/DANDI:<dandiset id>[/<version id>] (<version id> cannot be 'draft')
 - https://<server>[/api]/[#/]dandiset/<dandiset id>[/<version>][/files[?location=<path>]]
 - https://*dandiarchive-org.netlify.app/...
 - https://<server>[/api]/dandisets/<dandiset id>[/versions[/<version>]]
 - https://<server>[/api]/assets/<asset id>[/download]
 - https://<server>[/api]/dandisets/<dandiset id>/versions/<version>/assets/<asset id>[/download]
 - https://<server>[/api]/dandisets/<dandiset id>/versions/<version>/assets/?path=<path>
 - https://<server>[/api]/dandisets/<dandiset id>/versions/<version>/assets/?glob=<glob>
 - dandi://<instance name>/<dandiset id>[@<version>][/<path>]
 - https://<server>/...

❯ dandi instances
dandi:
  api: https://api.dandiarchive.org/api
  gui: https://dandiarchive.org
dandi-api-local-docker-tests:
  api: http://localhost:8000/api
  gui: http://localhost:8085
dandi-sandbox:
  api: https://api.sandbox.dandiarchive.org/api
  gui: https://sandbox.dandiarchive.org
dandi-staging:
  api: https://api.sandbox.dandiarchive.org/api
  gui: https://sandbox.dandiarchive.org
ember:
  api: https://api-dandi.emberarchive.org/api
  gui: https://dandi.emberarchive.org
ember-sandbox:
  api: https://api-dandi-sandbox.emberarchive.org/api
  gui: https://apl-setup--ember-dandi-archive.netlify.app/
linc:
  api: https://api.lincbrain.org/api
  gui: https://lincbrain.org
linc-staging:
  api: https://staging-api.lincbrain.org/api
  gui: https://staging.lincbrain.org

Adjust so it allows for any of those known/hardcoded instances . For documentation example it should just list <INSTANCE>:<dandiset id>[/<version>] instead of DANDI:<dandiset id>[/<version>]</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Adjust known_urls to support additional instances Support all known instances in short URL format Nov 21, 2025
Copilot AI requested a review from yarikoptic November 21, 2025 20:47
Copilot finished work on behalf of yarikoptic November 21, 2025 20:47
Copy link
Contributor Author

Copilot AI commented Nov 21, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api-dandi-sandbox.emberarchive.org
    • Triggering command: /usr/bin/python3 python3 (dns block)
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)
  • api-dandi.emberarchive.org
    • Triggering command: /usr/bin/python3 python3 (dns block)
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)
  • api.dandiarchive.org
    • Triggering command: /usr/bin/python3 python3 (dns block)
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)
  • api.lincbrain.org
    • Triggering command: /usr/bin/python3 python3 (dns block)
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)
  • api.sandbox.dandiarchive.org
    • Triggering command: /usr/bin/python3 python3 (dns block)
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)
  • dandiarchive.org
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)
  • sandbox.dandiarchive.org
    • Triggering command: /usr/bin/python3 python3 -m pytest dandi/tests/test_dandiarchive.py::test_parse_api_url -v --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of yarikoptic November 21, 2025 21:57
Copilot AI requested a review from yarikoptic November 21, 2025 21:57
@yarikoptic yarikoptic added minor Increment the minor version when merged enhancement New feature or request labels Nov 27, 2025
@yarikoptic
Copy link
Member

I vaguely recall we might have a listing of supported regexes dumped/hardcoded somewhere in docs. Please check and adjust if needed

@yarikoptic yarikoptic marked this pull request as ready for review November 27, 2025 02:38
@codecov
Copy link

codecov bot commented Nov 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.71%. Comparing base (c43062d) to head (3f49100).

❗ There is a different number of reports uploaded between BASE (c43062d) and HEAD (3f49100). Click for more details.

HEAD has 57 uploads less than BASE
Flag BASE (c43062d) HEAD (3f49100)
unittests 58 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1750       +/-   ##
===========================================
- Coverage   75.05%   52.71%   -22.34%     
===========================================
  Files          84       84               
  Lines       11873    11847       -26     
===========================================
- Hits         8911     6245     -2666     
- Misses       2962     5602     +2640     
Flag Coverage Δ
unittests 52.71% <100.00%> (-22.34%) ⬇️

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.

Copilot AI and others added 2 commits November 27, 2025 12:04
- Changed regex from generic `[-\w._]+` to pattern composed from known instance names
- Prevents false matches like `http:000001` or other invalid patterns
- Pattern is built dynamically: dandi|dandi-staging|dandi-sandbox|...|ember-sandbox
- All existing tests pass, invalid patterns now correctly rejected

Co-authored-by: yarikoptic <[email protected]>
@yarikoptic yarikoptic force-pushed the copilot/adjust-known-urls-support branch from 83173b1 to 3f49100 Compare November 27, 2025 17:04
@yarikoptic
Copy link
Member

@copilot I submitted for rebase, when CI finishes up - please review failures and whether relate to the changes - and where they do - fix them.

Copy link
Contributor Author

Copilot AI commented Nov 27, 2025

I vaguely recall we might have a listing of supported regexes dumped/hardcoded somewhere in docs. Please check and adjust if needed

Found and updated docs/source/ref/urls.rst (line 20) which had the hardcoded DANDI: format. It now documents the short URL format as :samp:\{instance-name}:{dandiset-id}[/{version}]`` with examples of known instances (DANDI, DANDI-SANDBOX, LINC, EMBER, etc.). Commit 684a1b0.

@copilot I submitted for rebase, when CI finishes up - please review failures and whether relate to the changes - and where they do - fix them.

CI is still pending for this branch after the rebase. I ran the related tests locally (test_parse_dandi_url_unknown_instance) and they pass. The documentation update I made in 684a1b0 should also build correctly on Read the Docs. Once CI runs complete, I'll review any failures related to these changes.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dandiarchive.org
    • Triggering command: /usr/bin/python python -m pytest dandi/tests/test_dandiarchive.py -v -k test_parse_dandi_url --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of yarikoptic November 27, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adjust known_urls to support known instances

2 participants