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 support for LINC staging and production APIs #1519

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions dandi/cli/tests/test_instances.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,21 @@ def test_cmd_instances(monkeypatch):
"dandi:\n"
" api: https://api.dandiarchive.org/api\n"
" gui: https://dandiarchive.org\n"
" is_private: null\n"
"dandi-api-local-docker-tests:\n"
f" api: http://{instancehost}:8000/api\n"
f" gui: http://{instancehost}:8085\n"
" is_private: null\n"
"dandi-staging:\n"
" api: https://api-staging.dandiarchive.org/api\n"
" gui: https://gui-staging.dandiarchive.org\n"
" is_private: null\n"
"linc:\n"
" api: https://api.lincbrain.org/api\n"
" gui: https://lincbrain.org\n"
" is_private: true\n"
"linc-staging:\n"
" api: https://staging-api.lincbrain.org/api\n"
" gui: https://staging.lincbrain.org\n"
" is_private: true\n"
)
15 changes: 15 additions & 0 deletions dandi/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
from enum import Enum
import os

from typing import Optional
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved

#: A list of metadata fields which dandi extracts from .nwb files.
#: Additional fields (such as ``number_of_*``) might be added by
#: `get_metadata()`
Expand Down Expand Up @@ -100,6 +102,7 @@ class DandiInstance:
name: str
gui: str | None
api: str
is_private: Optional[bool] = None
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved

@property
def redirector(self) -> None:
Expand Down Expand Up @@ -132,6 +135,18 @@ def urls(self) -> Iterator[str]:
f"http://{instancehost}:8085",
f"http://{instancehost}:8000/api",
),
"linc": DandiInstance(
"linc",
"https://lincbrain.org",
"https://api.lincbrain.org/api",
is_private=True
),
"linc-staging": DandiInstance(
"linc-staging",
"https://staging.lincbrain.org",
Copy link
Member

Choose a reason for hiding this comment

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

Hi @aaronkanzer, it looks like https://staging.lincbrain.org isn't able to resolve again. Not sure if we changed anything. See screenshot.

image

Copy link
Member Author

@aaronkanzer aaronkanzer Nov 12, 2024

Choose a reason for hiding this comment

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

Screenshot 2024-11-12 at 12 24 18 PM

@kabilar -- it seems that SSL support isn't provided out-of-the-box for the branch deploys URLs in Netlify

Firefox and Chrome require the SSL enforcement, but if you use Safari, you can proceed without the SSL -- didn't want to disturb the peace here with changing the SSL cert, so didn't update anything for now

"https://staging-api.lincbrain.org/api",
is_private=True
)
}
# to map back url: name
known_instances_rev = {
Expand Down
2 changes: 1 addition & 1 deletion dandi/tests/test_dandiarchive.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ def test_parse_dandi_url_unknown_instance() -> None:
parse_dandi_url("dandi://not-an-instance/000001")
assert str(excinfo.value) == (
"Unknown instance 'not-an-instance'. Valid instances: dandi,"
" dandi-api-local-docker-tests, dandi-staging"
" dandi-api-local-docker-tests, dandi-staging, linc, linc-staging"
Copy link
Member

Choose a reason for hiding this comment

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

might be worth right away to compose this from known_instances to avoid hardcoding. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just the unit test case -- in the prod. code it isn't hard-coding

Did you have in-mind the hard-coding being removed in the unit test here as well?

Copy link
Member

Choose a reason for hiding this comment

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

yeap. I see no reason to hardcode them in the test here. The purpose is to test to see that all are listed in the particular order. Code could be used to provide that and avoid future edits happen we need to add one more instance

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

yeap. I see no reason to hardcode them in the test here. The purpose is to test to see that all are listed in the particular order. Code could be used to provide that and avoid future edits happen we need to add one more instance

Updated the test here

)


Expand Down
28 changes: 14 additions & 14 deletions dandi/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -604,19 +604,19 @@
f"Could not retrieve server info from {url},"
" and client does not recognize URL"
)
try:
minversion = Version(server_info.cli_minimal_version)
bad_versions = [Version(v) for v in server_info.cli_bad_versions]
except ValueError as e:
raise ValueError(
f"{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)
# try:
# minversion = Version(server_info.cli_minimal_version)
# bad_versions = [Version(v) for v in server_info.cli_bad_versions]
# except ValueError as e:
# raise ValueError(
# f"{url} returned an incorrectly formatted version;"
# f" please contact that server's administrators: {e}"
Comment on lines +607 to +613

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
# )
# 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)
Comment on lines +616 to +619

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
api_url = server_info.services.api.url
if dandi_id is None:
# Don't use pydantic.AnyHttpUrl, as that sets the `port` attribute even
Expand All @@ -642,7 +642,7 @@


def is_url(s: str) -> bool:
"""Very primitive url detection for now
"""Very primitive url detection

TODO: redo
"""
Expand Down
6 changes: 6 additions & 0 deletions docs/source/cmdline/instances.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ Example output:
dandi-staging:
api: https://api-staging.dandiarchive.org/api
gui: https://gui-staging.dandiarchive.org
linc-staging:
api: https://staging-api.lincbrain.org/api
gui: https://staging.lincbrain.org
linc:
api: https://api.lincbrain.org/api
gui: https://lincbrain.org
Loading