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.

Sign up for GitHub

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

Jump to bottom

Add support for LINC staging and production APIs #1519

Closed
wants to merge 9 commits into from
Closed

Add support for LINC staging and production APIs #1519

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6d23a4b
Support LINC staging and production APIs in dandi-cli tooling
Nov 5, 2024
fce9ca9
temp
Nov 7, 2024
979c076
Resolve failing unit tests
Nov 8, 2024
5071abb
remove venv
Nov 8, 2024
f5a6f1f
Trivial change to invoke CI tests again
Nov 12, 2024
5628bc1
disable versioning for now
Nov 12, 2024
45360f9
remove blocking logic
Nov 12, 2024
3e258a4
remove is_private kwarg
Nov 12, 2024
aa080a1
update per yarik's comment
Nov 12, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
View file
Open in desktop
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.

on this note, chime in to

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
6 changes: 6 additions & 0 deletions docs/source/cmdline/instances.rst
View file
Open in desktop
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