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 all 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
6 changes: 6 additions & 0 deletions dandi/cli/tests/test_instances.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,10 @@ def test_cmd_instances(monkeypatch):
"dandi-staging:\n"
" api: https://api-staging.dandiarchive.org/api\n"
" gui: https://gui-staging.dandiarchive.org\n"
"linc:\n"
" api: https://api.lincbrain.org/api\n"
" gui: https://lincbrain.org\n"
"linc-staging:\n"
" api: https://staging-api.lincbrain.org/api\n"
" gui: https://staging.lincbrain.org\n"
)
10 changes: 10 additions & 0 deletions dandi/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ 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",
),
"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",
)
}
# to map back url: name
known_instances_rev = {
Expand Down
22 changes: 11 additions & 11 deletions dandi/dandiapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,17 +649,17 @@
schema_version = models.get_schema_version()
server_info = self.get("/info/")
server_schema_version = server_info.get("schema_version")
if not server_schema_version:
raise RuntimeError(
"Server did not provide schema_version in /info/;"
f" returned {server_info!r}"
)
if server_schema_version != schema_version:
raise SchemaVersionError(
f"Server requires schema version {server_schema_version};"
f" client only supports {schema_version}. You may need to"
" upgrade dandi and/or dandischema."
)
# if not server_schema_version:
Copy link
Member

Choose a reason for hiding this comment

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

why this block is commented out -- doesn't LINC provide /info/ with schema_version?

Copy link
Member Author

Choose a reason for hiding this comment

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

When I was testing, my forked branch is technically tagged as a version of 0+untagged<some-sha-here> thus I commented this out for testing purposes.

As noted in #1519 (comment) -- this will be commented back in once you et. al are happy with the core changes here

# raise RuntimeError(
# "Server did not provide schema_version in /info/;"
# f" returned {server_info!r}"
Comment on lines +652 to +655

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines +652 to +655
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# if not server_schema_version:
# raise RuntimeError(
# "Server did not provide schema_version in /info/;"
# f" returned {server_info!r}"
if not server_schema_version:
raise RuntimeError(
"Server did not provide schema_version in /info/;"
f" returned {server_info!r}"

# )
# if server_schema_version != schema_version:
# raise SchemaVersionError(
# f"Server requires schema version {server_schema_version};"
# f" client only supports {schema_version}. You may need to"
# " upgrade dandi and/or dandischema."
Comment on lines +657 to +661

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment for reference in another issue

# )
Comment on lines +656 to +662
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# )
# if server_schema_version != schema_version:
# raise SchemaVersionError(
# f"Server requires schema version {server_schema_version};"
# f" client only supports {schema_version}. You may need to"
# " upgrade dandi and/or dandischema."
# )
)
if server_schema_version != schema_version:
raise SchemaVersionError(
f"Server requires schema version {server_schema_version};"
f" client only supports {schema_version}. You may need to"
" upgrade dandi and/or dandischema."
)


def get_asset(self, asset_id: str) -> BaseRemoteAsset:
"""
Expand Down
9 changes: 5 additions & 4 deletions dandi/tests/test_dandiarchive.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,11 @@
def test_parse_dandi_url_unknown_instance() -> None:
with pytest.raises(UnknownURLError) as excinfo:
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"
)

valid_instances = ", ".join(sorted(known_instances.keys()))
expected_message = f"Unknown instance 'not-an-instance'. Valid instances: {valid_instances}"

Check warning on line 444 in dandi/tests/test_dandiarchive.py

View check run for this annotation

Codecov / codecov/patch

dandi/tests/test_dandiarchive.py#L443-L444

Added lines #L443 - L444 were not covered by tests

assert str(excinfo.value) == expected_message

Check warning on line 446 in dandi/tests/test_dandiarchive.py

View check run for this annotation

Codecov / codecov/patch

dandi/tests/test_dandiarchive.py#L446

Added line #L446 was not covered by tests


@mark.skipif_no_network
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