Skip to content

Commit

Permalink
Merge pull request #565 from dandi/gh-541
Browse files Browse the repository at this point in the history
Switch default dandi instance to dandi-api based on redirector
  • Loading branch information
yarikoptic committed Apr 15, 2021
2 parents 6a5f285 + 53b2021 commit 90049f8
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 20 deletions.
22 changes: 20 additions & 2 deletions dandi/dandiarchive.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ class _dandi_url_parser:
# - server_type:
# - 'girder' - underlying requests should go to girder server
# - 'api' - the "new" API service
# - 'redirect' - use redirector's server-info
# - rewrite:
# - callable -- which would rewrite that "URI"
# - map_instance
Expand Down Expand Up @@ -223,7 +224,7 @@ class _dandi_url_parser:
r"(/(?P<version>draft))?"
rf"(/files(\?_id={id_grp}(&_modelType=folder)?)?)?"
),
{"server_type": "girder"},
{"server_type": "redirect"},
"https://<server>[/api]#/dandiset/<dandiset id>[/draft]"
"[/files[?_id=<id>[&_modelType=folder]]]",
),
Expand Down Expand Up @@ -375,7 +376,24 @@ def parse(cls, url, *, map_instance=True):
)

url_server = groups["server"]
server = cls.map_to[server_type].get(url_server.rstrip("/"), url_server)
if server_type == "redirect":
try:
instance_name = known_instances_rev[url_server.rstrip("/")]
except KeyError:
raise UnknownURLError(f"{url} does not map to a known instance")
instance = get_instance(instance_name)
if instance.metadata_version == 0:
server_type = "girder"
server = instance.girder
elif instance.metadata_version == 1:
server_type = "api"
server = instance.api
else:
raise RuntimeError(
f"Unknown instance metadata_version: {instance.metadata_version}"
)
else:
server = cls.map_to[server_type].get(url_server.rstrip("/"), url_server)

if not server.endswith("/"):
server += "/" # we expected '/' to be there so let it be
Expand Down
6 changes: 3 additions & 3 deletions dandi/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
import click
import requests

from .consts import dandiset_metadata_file, known_instances
from .consts import dandiset_metadata_file
from .dandiapi import DandiAPIClient
from .dandiarchive import parse_dandi_url
from .exceptions import NotFoundError
from .utils import is_url
from .utils import get_instance, is_url


class RemoteAsset(NamedTuple):
Expand Down Expand Up @@ -115,7 +115,7 @@ def register_url(self, url: str) -> None:
raise RuntimeError(f"Unexpected asset type for {url}: {asset_type}")

def register_local_path_equivalent(self, instance_name: str, filepath: str) -> None:
instance = known_instances[instance_name]
instance = get_instance(instance_name)
if instance.girder is not None:
raise NotImplementedError("Cannot delete assets from Girder instances")
api_url = instance.api
Expand Down
51 changes: 51 additions & 0 deletions dandi/tests/test_dandiarchive.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
import responses

from dandi.consts import known_instances
from dandi.dandiarchive import follow_redirect, parse_dandi_url
Expand Down Expand Up @@ -174,3 +175,53 @@ def test_follow_redirect():
follow_redirect("https://bit.ly/dandi12")
== "https://gui.dandiarchive.org/#/file-browser/folder/5e72b6ac3da50caa9adb0498"
)


@responses.activate
def test_parse_gui_old_redirect():
responses.add(
responses.GET,
"https://dandiarchive.org/server-info",
json={
"version": "1.2.0",
"cli-minimal-version": "0.6.0",
"cli-bad-versions": [],
"services": {
"girder": {"url": "https://girder.dandiarchive.org"},
"webui": {"url": "https://gui.dandirchive.org"},
"api": None,
"jupyterhub": {"url": "https://hub.dandiarchive.org"},
},
},
)
assert parse_dandi_url("https://gui.dandiarchive.org/#/dandiset/000003") == (
"girder",
"https://girder.dandiarchive.org/",
"dandiset",
{"dandiset_id": "000003", "version": "draft"},
)


@responses.activate
def test_parse_gui_new_redirect():
responses.add(
responses.GET,
"https://dandiarchive.org/server-info",
json={
"version": "1.2.0",
"cli-minimal-version": "0.6.0",
"cli-bad-versions": [],
"services": {
"girder": None,
"webui": {"url": "https://gui.dandirchive.org"},
"api": {"url": "https://api.dandiarchive.org/api"},
"jupyterhub": {"url": "https://hub.dandiarchive.org"},
},
},
)
assert parse_dandi_url("https://gui.dandiarchive.org/#/dandiset/000003") == (
"api",
"https://api.dandiarchive.org/api/",
"dandiset",
{"dandiset_id": "000003", "version": None},
)
40 changes: 33 additions & 7 deletions dandi/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def test_get_instance_dandi():
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"api": None,
"jupyterhub": {"url": "https://hub.dandi"},
},
},
Expand All @@ -178,7 +178,33 @@ def test_get_instance_dandi():
girder="https://girder.dandi",
gui="https://gui.dandi",
redirector="https://dandiarchive.org",
api="https://publish.dandi/api",
api=None,
)


@responses.activate
def test_get_instance_dandi_with_api():
responses.add(
responses.GET,
"https://dandiarchive.org/server-info",
json={
"version": "1.0.0",
"cli-minimal-version": "0.5.0",
"cli-bad-versions": [],
"services": {
"girder": None,
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://api.dandi"},
"jupyterhub": {"url": "https://hub.dandi"},
},
},
)
assert get_instance("dandi") == dandi_instance(
metadata_version=1,
girder=None,
gui="https://gui.dandi",
redirector="https://dandiarchive.org",
api="https://api.dandi",
)


Expand All @@ -194,7 +220,7 @@ def test_get_instance_url():
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"api": None,
"jupyterhub": {"url": "https://hub.dandi"},
},
},
Expand All @@ -204,7 +230,7 @@ def test_get_instance_url():
girder="https://girder.dandi",
gui="https://gui.dandi",
redirector="https://example.dandi/",
api="https://publish.dandi/api",
api=None,
)


Expand All @@ -220,7 +246,7 @@ def test_get_instance_cli_version_too_old():
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"api": None,
"jupyterhub": {"url": "https://hub.dandi"},
},
},
Expand All @@ -245,7 +271,7 @@ def test_get_instance_bad_cli_version():
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"api": None,
"jupyterhub": {"url": "https://hub.dandi"},
},
},
Expand Down Expand Up @@ -312,7 +338,7 @@ def test_get_instance_bad_version_from_server():
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"api": None,
"jupyterhub": {"url": "https://hub.dandi"},
},
},
Expand Down
29 changes: 21 additions & 8 deletions dandi/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,14 +618,27 @@ def get_instance(dandi_instance_id):
raise CliVersionTooOldError(our_version, minversion, bad_versions)
if our_version in bad_versions:
raise BadCliVersionError(our_version, minversion, bad_versions)
services = server_info["services"]
return dandi_instance(
metadata_version=0,
girder=services.get("girder", {}).get("url"),
gui=services.get("webui", {}).get("url"),
redirector=redirector_url,
api=services.get("api", {}).get("url"),
)
if server_info["services"].get("girder"):
return dandi_instance(
metadata_version=0,
girder=server_info["services"].get("girder", {}).get("url"),
gui=server_info["services"].get("webui", {}).get("url"),
redirector=redirector_url,
api=None,
)
elif server_info["services"].get("api"):
return dandi_instance(
metadata_version=1,
girder=None,
gui=server_info["services"].get("webui", {}).get("url"),
redirector=redirector_url,
api=server_info["services"].get("api", {}).get("url"),
)
else:
raise RuntimeError(
"redirector's server-info returned unknown set of services keys: "
+ ", ".join(k for k, v in server_info["services"].items() if v is not None)
)


TITLE_CASE_LOWER = {
Expand Down

0 comments on commit 90049f8

Please sign in to comment.