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

use redirector's /server-info (or just "OPTIONS" call) #184

Closed
yarikoptic opened this issue Aug 10, 2020 · 0 comments · Fixed by #186
Closed

use redirector's /server-info (or just "OPTIONS" call) #184

yarikoptic opened this issue Aug 10, 2020 · 0 comments · Fixed by #186
Assignees
@jwodder

Comments

@yarikoptic
Copy link
Member

A satellite issue to redirector's dandi/redirector#20 to be merged in after redirector gets its portion merged:

  • provide a helper (probably with caching) e.g. get_instance to avoid direct use of known_instances lookup table. Instead it should given an instance name (or even just a URL later on?), take a .redirector url and request that information from it. If it is a URL provided -- should just talk to it directly (that would in the future allow to specify "instance" via URL not hardcoded name)
  • if there is a response from the redirector providing the record:
    • if current client version does not satisfy cli-version in the record, raise a dedicated exception which would explain that the instance demands minimal version X and there are some bad versions which must not be used (if any prescribed in that record).
    • compose instance record using .url fields from the record (instead of hardcoded ones in consts.py)
  • if there is no response from the redirector, issue a warning and continue using hard-coded values for now. later we will RF it away but probably worth keeping it as a safety blanket for now.
jwodder added a commit that referenced this issue Aug 10, 2020
@jwodder
[GH-184] Add a get_instance() function
a555b2b
jwodder added a commit that referenced this issue Aug 10, 2020
@jwodder
[GH-184] Add a get_instance() function
9c0a6df
jwodder added a commit that referenced this issue Aug 10, 2020
@jwodder
[GH-184] Add a get_instance() function
38c3047
jwodder added a commit that referenced this issue Aug 10, 2020
@jwodder
[GH-184] Add a get_instance() function
20c8650
@yarikoptic yarikoptic mentioned this issue Aug 11, 2020
Closed
@jwodder jwodder mentioned this issue Aug 11, 2020
Merged
yarikoptic added a commit that referenced this issue Aug 12, 2020
@yarikoptic
Merge pull request #186 from dandi/gh-184
3137916 
Add a get_instance() function
yarikoptic added a commit that referenced this issue Aug 12, 2020
@yarikoptic
Merge remote-tracking branch 'origin/master' into enh-download-api
19e3d77 
* origin/master: (24 commits)
  Remove duplicate "requests" dependency
  get_instance(): Immediately return known instances that lack redirector URLs
  Raise helpful error if /server-info returns invalid version
  Make redirector report http://localhost:8081 as Girder URL
  Test uploading an unregistered dandiset
  Assert a certain code path is unreachable
  Test that lock_dandiset() is called when uploading
  Tests of lock_dandiset()
  Remove unused monkeypatches
  Configure flake8 to run on test code as well
  Detect nonexistent Dandisets when uploading
  ENH: upload - add locking of the dandiset
  Make pytest ignore DeprecationWarnings from responses
  Fetch all commits in test workflow
  Replace some occurrences of known_instances with get_instance()
  Test get_instance()
  CliVersionTooLowError → CliVersionTooOldError
  Move CLI version exceptions to dandi/exceptions.py
  Use semantic_version instead of packaging
  [GH-184] Add a get_instance() function
  ...

 Conflicts:
	dandi/download.py
	dandi/girder.py
	dandi/tests/test_download.py - conflicted with a linting change
	dandi/tests/test_utils.py - both added tests

 No conflicts but needed changes:
    dandi/utils.py -- needed to add dandiapi into returned dandi_instance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jwodder jwodder

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add a get_instance() function dandi/dandi-cli
2 participants
@yarikoptic @jwodder