Add support for LINC staging and production APIs #1519

aaronkanzer · 2024-11-05T19:43:51Z

Relates to #1517 -- please reference #1527

Cc @kabilar -- still very much in draft mode as I need to evaluate the scope of failing tests across the DANDI ecosystem

codecov · 2024-11-05T19:56:28Z

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.01%. Comparing base (6aa414c) to head (aa080a1).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
dandi/tests/test_dandiarchive.py	0.00%	3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (6aa414c) and HEAD (aa080a1). Click for more details.

HEAD has 150 uploads less than BASE

Flag BASE (6aa414c) HEAD (aa080a1)

unittests 151 1

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1519       +/-   ##
===========================================
- Coverage   88.58%   68.01%   -20.57%     
===========================================
  Files          78       78               
  Lines       10589    10562       -27     
===========================================
- Hits         9380     7184     -2196     
- Misses       1209     3378     +2169

Flag	Coverage Δ
unittests	`68.01% <0.00%> (-20.57%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kabilar · 2024-11-10T01:31:42Z

dandi/consts.py

+    ),
+    "linc-staging": DandiInstance(
+        "linc-staging",
+        "https://staging.lincbrain.org",


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

@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

dandi/utils.py

+    # 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}"


dandi/utils.py

+    # if our_version < minversion:
+    #     raise CliVersionTooOldError(our_version, minversion, bad_versions)
+    # if our_version in bad_versions:
+    #     raise BadCliVersionError(our_version, minversion, bad_versions)


dandi/dandiapi.py

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


aaronkanzer · 2024-11-15T14:22:17Z

dandi/dandiapi.py

+        # 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 for reference in another issue

aaronkanzer · 2024-11-12T18:43:56Z

@kabilar @yarikoptic @satra @waxlamp -- just did a full end-to-end test....and it works! It seems we should be safe to sunset lincbrain-cli and include other DANDI instances similar to how we have done here 😄

I had to comment out some version validation code in the PR since my test branch isn't semantic versioned yet (e.g. if you look at the code, you'll see that those are commented out -- thus keeping in PR draft status for now). I figure once I get a review here on the relevant code, I can uncomment out the version validation.

Here is how I confirmed that the branch was succcessful:

pip uninstall dandi -y to remove DANDI CLI locally (can use a venv too if desired)
pip install -e git+https://github.com/aaronkanzer/dandi-cli.git@ak-abstraction#egg=dandi -- reference my forked branch here as the dandi CLI version
Made a Dandiset on LINC: https://lincbrain.org/dandiset/000044
Run dandi download https://lincbrain.org/dandiset/000044/draft -- the CLI should prompt for a LINCBRAIN_API_KEY value here
I copied over an NWB into the 000044 folder
Run dandi upload -i linc --validation skip
Observe files present in https://lincbrain.org/dandiset/000044/draft/files?location=

Prior to running any steps, I ran dandi instances to see that the proper values were listed

One other note: It seems that the authentication works as intended fortunately, so the is_private bool shouldn't be necessary -- unless there are other mechanisms where we might want it -- @yarikoptic perhaps a code review whenever you have the chance to see if there might be any utility in Ember -- I think I can/should remove it however -- thanks all

dandi/consts.py

yarikoptic

Great to hear that this might be all what is needed to support LINC!

From the quick review I do not see where is_private even used, thus questioning on the need for it.

dandi/consts.py

yarikoptic · 2024-11-12T20:38:03Z

dandi/dandiapi.py

-                f" client only supports {schema_version}.  You may need to"
-                " upgrade dandi and/or dandischema."
-            )
+        # if not server_schema_version:


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

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

yarikoptic · 2024-11-12T20:38:58Z

dandi/tests/test_dandiarchive.py

@@ -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"


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

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?

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

on this note, chime in to

Add configuration system #1521

Define/maintain centralized registry of instances #1522

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

dandi/consts.py

aaronkanzer · 2024-11-12T21:00:24Z

Great to hear that this might be all what is needed to support LINC!

From the quick review I do not see where is_private even used, thus questioning on the need for it.

Awesome -- yeah, I kept the is_private flag just to see if it could have some utility somehow for Ember (I don't know the requirements there, so just deferring to you)

I've removed it in my most recent commit -- Cc @jwodder

kabilar · 2024-11-13T18:02:18Z

Run dandi download https://lincbrain.org/dandiset/000044/draft -- the CLI should prompt for a LINCBRAIN_API_KEY value here

Just confirming, we are asking users to declare the API key for LINC using the DANDI_API_KEY environment variable. Is this correct?

kabilar · 2024-11-13T21:20:24Z

dandi/dandiapi.py

+        #     )
+        # 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."
+        #     )


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."

)

kabilar · 2024-11-13T21:20:38Z

dandi/dandiapi.py

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


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}"

aaronkanzer · 2024-11-15T14:24:50Z

@yarikoptic @jwodder -- mind doing another re-review? Do you want me to uncomment out the version validation code first?

(I made this Issue to observe this workaround for now -- #1526)

Thanks in advance

aaronkanzer · 2024-11-15T15:50:09Z

@yarikoptic @jwodder @kabilar -- closing this Issue because of the release tagging missing in this fork -- #1527 should be the code merged -- thanks again @jwodder

Support LINC staging and production APIs in dandi-cli tooling

6d23a4b

kabilar changed the title ~~Support LINC staging and production APIs in dandi-cli tooling~~ Add support for LINC staging and production APIs Nov 5, 2024

temp

fce9ca9

kabilar mentioned this pull request Nov 7, 2024

Add docs framework for how to clone a new DANDI instance dandi/handbook#104

Open

Aaron Kanzer added 2 commits November 8, 2024 15:31

Resolve failing unit tests

979c076

remove venv

5071abb

kabilar reviewed Nov 10, 2024

View reviewed changes

Aaron Kanzer added 2 commits November 12, 2024 12:27

Trivial change to invoke CI tests again

f5a6f1f

disable versioning for now

5628bc1

github-advanced-security bot found potential problems Nov 12, 2024

View reviewed changes

remove blocking logic

45360f9

github-advanced-security bot found potential problems Nov 12, 2024

View reviewed changes

This was referenced Nov 12, 2024

Upgrade dandischema to 0.10.4, Update UI references to dandi CLI instead of lincbrain CLI lincbrain/linc-archive#276

Merged

Remove lincbrain CLI installation due to dandi CLI handling different APIs dandi/dandi-hub#205

Merged

yarikoptic requested a review from jwodder November 12, 2024 20:23

jwodder reviewed Nov 12, 2024

View reviewed changes

dandi/consts.py Outdated Show resolved Hide resolved

yarikoptic requested changes Nov 12, 2024

View reviewed changes

remove is_private kwarg

3e258a4

github-advanced-security bot found potential problems Nov 12, 2024

View reviewed changes

dandi/consts.py Fixed Show resolved Hide resolved

yarikoptic mentioned this pull request Nov 12, 2024

Define/maintain centralized registry of instances #1522

Open

update per yarik's comment

aa080a1

kabilar mentioned this pull request Nov 13, 2024

Fix unit tests lincbrain/linc-cli#57

Closed

1 task

kabilar reviewed Nov 13, 2024

View reviewed changes

aaronkanzer mentioned this pull request Nov 15, 2024

Create option to bypass untagged CLI versions to integration test code properly #1526

Closed

aaronkanzer closed this Nov 15, 2024

aaronkanzer mentioned this pull request Nov 15, 2024

Include LINC API as a supported instance type for DANDI CLI #1527

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for LINC staging and production APIs #1519

Add support for LINC staging and production APIs #1519

aaronkanzer commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 5, 2024 •

edited

Loading

kabilar Nov 10, 2024

aaronkanzer Nov 12, 2024 •

edited

Loading

aaronkanzer Nov 15, 2024

aaronkanzer commented Nov 12, 2024 •

edited

Loading

yarikoptic left a comment

yarikoptic Nov 12, 2024

aaronkanzer Nov 12, 2024

yarikoptic Nov 12, 2024

aaronkanzer Nov 12, 2024

yarikoptic Nov 12, 2024

yarikoptic Nov 12, 2024

aaronkanzer Nov 12, 2024

aaronkanzer commented Nov 12, 2024

kabilar commented Nov 13, 2024

kabilar Nov 13, 2024

kabilar Nov 13, 2024

aaronkanzer commented Nov 15, 2024

aaronkanzer commented Nov 15, 2024

Add support for LINC staging and production APIs #1519

Add support for LINC staging and production APIs #1519

Conversation

aaronkanzer commented Nov 5, 2024 • edited Loading

codecov bot commented Nov 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

aaronkanzer Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronkanzer commented Nov 12, 2024 • edited Loading

yarikoptic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronkanzer commented Nov 12, 2024

kabilar commented Nov 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronkanzer commented Nov 15, 2024

aaronkanzer commented Nov 15, 2024

aaronkanzer commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 5, 2024 •

edited

Loading

aaronkanzer Nov 12, 2024 •

edited

Loading

aaronkanzer commented Nov 12, 2024 •

edited

Loading