-
Notifications
You must be signed in to change notification settings - Fork 29
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
move: Immediately authenticate client after creation #1480
Conversation
In order to deal with embargoed Dandisets
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1480 +/- ##
==========================================
+ Coverage 88.52% 88.60% +0.08%
==========================================
Files 77 77
Lines 10560 10561 +1
==========================================
+ Hits 9348 9358 +10
+ Misses 1212 1203 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@yarikoptic Ready for review. |
@@ -812,6 +812,7 @@ def move( | |||
raise TypeError("`dandiset` must be a Path when work_on='both'") | |||
local_ds, subpath = find_dandiset_and_subpath(dandiset) | |||
client = DandiAPIClient.for_dandi_instance(dandi_instance) | |||
client.dandi_authenticate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in principle we do not need to authenticate if doing dry_run
do we?
Would it be feasible in general to provide logic to authenticate when receiving 401 and haven't been authenticated before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authentication is still needed for dry runs so that the list of assets can be fetched for an embargoed Dandiset.
Thank you, let's proceed! |
🚀 PR was released in |
In order to deal with embargoed Dandisets
Closes #1478.