Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a get_instance() function #186

Merged
merged 11 commits into from
Aug 12, 2020
6 changes: 4 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ jobs:
steps:
- name: Set up environment
uses: actions/checkout@v1
with: # no need for the history
fetch-depth: 1
with:
# Fetch all commits so that versioneer will return something compatible
# with semantic-version
fetch-depth: 0
- name: Set up Python ${{ matrix.python }}
uses: actions/setup-python@v1
with:
Expand Down
6 changes: 2 additions & 4 deletions dandi/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from .consts import dandiset_metadata_file, known_instances, metadata_digests
from .dandiset import Dandiset
from .exceptions import FailedToConnectError, NotFoundError, UnknownURLError
from .utils import flatten, flattened, Parallel, delayed
from .utils import flatten, flattened, Parallel, delayed, get_instance

lgr = get_logger()

Expand Down Expand Up @@ -121,9 +121,7 @@ def parse(cls, url, *, map_instance=True):
settings["map_instance"], ", ".join(known_instances)
)
)
return (known_instances[settings["map_instance"]].girder,) + tuple(
_
)
return (get_instance(settings["map_instance"]).girder,) + tuple(_)
continue # in this run we ignore an match further
else:
break
Expand Down
31 changes: 31 additions & 0 deletions dandi/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,34 @@ class FailedToConnectError(RuntimeError):
"""Failed to connect to online resource"""

pass


class CliVersionError(RuntimeError):
""" Base class for `CliVersionTooOldError` and `BadCliVersionError` """

def __init__(self, our_version, minversion, bad_versions):
self.our_version = our_version
self.minversion = minversion
self.bad_versions = bad_versions

def server_requirements(self):
s = f"Server requires at least version {self.minversion}"
if self.bad_versions:
s += f" (but not {', '.join(map(str, self.bad_versions))})"
return s


class CliVersionTooOldError(CliVersionError):
def __str__(self):
return (
f"Client version {self.our_version} is too old! "
+ self.server_requirements()
)


class BadCliVersionError(CliVersionError):
def __str__(self):
return (
f"Client version {self.our_version} is rejected by server! "
+ self.server_requirements()
)
5 changes: 3 additions & 2 deletions dandi/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@

from . import get_logger
from . import girder
from .consts import dandiset_metadata_file, known_instances, routes
from .consts import dandiset_metadata_file, routes
from .dandiset import Dandiset
from .utils import get_instance

lgr = get_logger()


def register(name, description, dandiset_path=None, dandi_instance="dandi"):
dandi_instance = known_instances[dandi_instance]
dandi_instance = get_instance(dandi_instance)
if not dandiset_path and op.exists(dandiset_metadata_file):
dandiset = Dandiset.find(os.getcwd())
if dandiset:
Expand Down
2 changes: 1 addition & 1 deletion dandi/tests/data/dandiarchive-docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ services:
ports:
- "8079:8080"
environment:
GIRDER_URL: http://girder:8080
GIRDER_URL: http://localhost:8081
GUI_URL: http://localhost:8086
ABOUT_URL: http://www.dandiarchive.org

Expand Down
171 changes: 171 additions & 0 deletions dandi/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@
import time
import datetime
import pytest
import responses
from .. import __version__
from ..consts import dandi_instance, known_instances
from ..exceptions import BadCliVersionError, CliVersionTooOldError
from ..utils import (
ensure_datetime,
ensure_strtime,
find_files,
flatten,
flattened,
get_instance,
get_utcnow_datetime,
is_same_time,
on_windows,
Expand Down Expand Up @@ -122,3 +127,169 @@ def test_flatten():
0,
1,
]


@responses.activate
def test_get_instance_dandi():
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": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"jupyterhub": {"url": "https://hub.dandi"},
},
},
)
assert get_instance("dandi") == dandi_instance(
girder="https://girder.dandi",
gui="https://gui.dandi",
redirector="https://dandiarchive.org",
)


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


@responses.activate
def test_get_instance_cli_version_too_old():
responses.add(
responses.GET,
"https://example.dandi/server-info",
json={
"version": "1.0.0",
"cli-minimal-version": "99.99.99",
"cli-bad-versions": [],
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"jupyterhub": {"url": "https://hub.dandi"},
},
},
)
with pytest.raises(CliVersionTooOldError) as excinfo:
get_instance("https://example.dandi/")
assert str(excinfo.value) == (
f"Client version {__version__} is too old!"
" Server requires at least version 99.99.99"
)


@responses.activate
def test_get_instance_bad_cli_version():
responses.add(
responses.GET,
"https://example.dandi/server-info",
json={
"version": "1.0.0",
"cli-minimal-version": "0.5.0",
"cli-bad-versions": [__version__],
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"jupyterhub": {"url": "https://hub.dandi"},
},
},
)
with pytest.raises(BadCliVersionError) as excinfo:
get_instance("https://example.dandi/")
assert str(excinfo.value) == (
f"Client version {__version__} is rejected by server!"
f" Server requires at least version 0.5.0 (but not {__version__})"
)


@responses.activate
def test_get_instance_id_bad_response():
responses.add(
responses.GET,
"https://dandiarchive.org/server-info",
body="404 -- not found",
status=404,
)
assert get_instance("dandi") is known_instances["dandi"]


@responses.activate
def test_get_instance_known_url_bad_response():
responses.add(
responses.GET,
"https://dandiarchive.org/server-info",
body="404 -- not found",
status=404,
)
assert get_instance("https://dandiarchive.org") is known_instances["dandi"]


@responses.activate
def test_get_instance_unknown_url_bad_response():
responses.add(
responses.GET,
"https://dandi.nil/server-info",
body="404 -- not found",
status=404,
)
with pytest.raises(RuntimeError) as excinfo:
get_instance("https://dandi.nil")
assert str(excinfo.value) == (
"Could not retrieve server info from https://dandi.nil,"
" and client does not recognize URL"
)


def test_get_instance_id_no_redirector():
assert get_instance("local-girder-only") is known_instances["local-girder-only"]


@responses.activate
def test_get_instance_bad_version_from_server():
responses.add(
responses.GET,
"https://example.dandi/server-info",
json={
"version": "1.0.0",
"cli-minimal-version": "foobar",
"cli-bad-versions": [],
"services": {
"girder": {"url": "https://girder.dandi"},
"webui": {"url": "https://gui.dandi"},
"api": {"url": "https://publish.dandi/api"},
"jupyterhub": {"url": "https://hub.dandi"},
},
},
)
with pytest.raises(ValueError) as excinfo:
get_instance("https://example.dandi/")
assert str(excinfo.value).startswith(
"https://example.dandi/ returned an incorrectly formatted version;"
" please contact that server's administrators: "
)
assert "foobar" in str(excinfo.value)
5 changes: 2 additions & 3 deletions dandi/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@

from .cli.command import lgr
from . import __version__
from .utils import ensure_datetime, ensure_strtime
from .utils import ensure_datetime, ensure_strtime, get_instance
from .consts import (
collection_drafts,
dandiset_identifier_regex,
dandiset_metadata_file,
known_instances,
metadata_digests,
)

Expand Down Expand Up @@ -91,7 +90,7 @@ def upload(

ignore_benign_pynwb_warnings() # so validate doesn't whine

client = girder.get_client(known_instances[dandi_instance].girder)
client = girder.get_client(get_instance(dandi_instance).girder)

try:
collection_rec = girder.ensure_collection(client, girder_collection)
Expand Down
53 changes: 53 additions & 0 deletions dandi/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@

from pathlib import Path

import requests
import ruamel.yaml
from semantic_version import Version

from . import __version__
from .consts import dandi_instance, known_instances, known_instances_rev
from .exceptions import BadCliVersionError, CliVersionTooOldError

if sys.version_info[:2] < (3, 7):
import dateutil.parser
Expand Down Expand Up @@ -504,3 +510,50 @@ def delayed(*args, **kwargs):
import joblib

return joblib.delayed(*args, **kwargs)


def get_instance(dandi_instance_id):
if dandi_instance_id.lower().startswith(("http://", "https://")):
redirector_url = dandi_instance_id
dandi_id = known_instances_rev.get(redirector_url)
if dandi_id is not None:
instance = known_instances[dandi_id]
else:
instance = None
else:
instance = known_instances[dandi_instance_id]
redirector_url = instance.redirector
if redirector_url is None:
return instance
try:
r = requests.get(redirector_url.rstrip("/") + "/server-info")
r.raise_for_status()
except Exception as e:
lgr.warning("Request to %s failed (%s)", redirector_url, str(e))
if instance is not None:
lgr.warning("Using hard-coded URLs")
return instance
else:
raise RuntimeError(
f"Could not retrieve server info from {redirector_url},"
" and client does not recognize URL"
)
server_info = r.json()
try:
minversion = Version(server_info["cli-minimal-version"])
bad_versions = [Version(v) for v in server_info["cli-bad-versions"]]
except ValueError as e:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking out loud: sorry for being slow, it got me thinking (since we do not test for version yet etc), what if any of those two is missing? and it kinda makes sense if server does not demand any specific version of the client somehow... I think later some time we might want to allow them to be "optional" but for now we would demand them, and if not present -- blow informatively... or let's just add that later ;)

raise ValueError(
f"{redirector_url} returned an incorrectly formatted version;"
f" please contact that server's administrators: {e}"
)
our_version = Version(__version__)
if our_version < minversion:
raise CliVersionTooOldError(our_version, minversion, bad_versions)
if our_version in bad_versions:
raise BadCliVersionError(our_version, minversion, bad_versions)
return dandi_instance(
girder=server_info["services"]["girder"]["url"],
gui=server_info["services"]["webui"]["url"],
redirector=redirector_url,
)
3 changes: 2 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ install_requires =
joblib
pyout
humanize
requests ~= 2.20
ruamel.yaml >=0.15, <1
keyring
keyrings.alt
Expand Down Expand Up @@ -70,7 +71,7 @@ test =
mock
pytest
pytest-cov
requests ~= 2.20
responses
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved
all =
#%(doc)s
%(extensions)s
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ filterwarnings =
error
ignore:No cached namespaces found .*:UserWarning
ignore:ignoring namespace '.*' because it already exists:UserWarning
ignore::DeprecationWarning:responses

[coverage:run]
parallel = True
Expand Down