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

Q: provide /server-info "natively" (or redirected to) by web service? #1108

Closed
yarikoptic opened this issue May 26, 2022 · 7 comments · Fixed by #1190
Closed

Q: provide /server-info "natively" (or redirected to) by web service? #1108

yarikoptic opened this issue May 26, 2022 · 7 comments · Fixed by #1190
Assignees
Labels
released This issue/pull request has been released.

Comments

@yarikoptic
Copy link
Member

ATM it is handled via redirect on netlify level

apiUrl = apiUrl.endsWith('/') ? apiUrl.slice(0, -1) : apiUrl;

so it is not available in docker-compose'd instance of the archive, thus no other but the netlify instances would have it. But dandi-cli uses /server-info to identify the actual endpoints for e.g. API service etc, so that is what it accesses whenever given some URL. So it would be nice to have that endpoint also for docker-compose instances. We have been relying on redirector before, so was tested and now we need to adjust the test dandi/dandi-cli#977 accordingly but there is no such endpoint in docker-compose setup.

@waxlamp
Copy link
Member

waxlamp commented Jul 6, 2022

@yarikoptic, do you know why we need this top-level URL at all? This simply redirects (in the case of the production instance) to https://api.dandiarchive.org/api/info/.

If it is because there is some bootstrapping needed to know that the API is served from /api on a specific host, it seems we could dispense with that and instead teach dandi-cli that https://api.dandiarchive.org/api/info/ is the canonical info point for production, so-and-so for staging, and for all other instances you would simply have to know the server URL (which is not too bad; currently you already need to know the client URL). Otherwise, it seems we would be going out of our way to duplicate this information on the client side, when it probably doesn't absolutely need to be there.

Thoughts?

@yarikoptic
Copy link
Member Author

yarikoptic commented Jul 6, 2022

The point of having each DANDI instance (be it main deployment, or staging, or my lab's, or some test docker) to provide /server-info so that dandi-cli when asked to operate on URLs coming from that main instance , e.g. dandi download https://dandiarchive.org/dandiset/000108, could just ask that particular instance about URLs for its services and not need to rely to carry with itself the knowledge of all instances of DANDI.

edit: note - users should not know about existence of some separate API server

@yarikoptic
Copy link
Member Author

Note: As the extended description of the #1190 says that PR added /server-info/ not /server-info (I have adjusted the title of that PR now to correspond). The original issue here asks for /server-info (without trailing /) and serving /server-info/ does not provide remedy (as picked up by @jwodder in dandi/dandi-cli#1020 (comment)). I am postponing reopening since may be we should just workaround on dandi-cli, discussion will continue in aforementioned issue.

@waxlamp
Copy link
Member

waxlamp commented Aug 3, 2022

If you could add the trailing slash in the CLI, that would be great, because it would solve your problem in the short term.

I know that we are in the middle of discussing this in #1200, but I think the solution proposed in that issue is the correct solution here, and will obviate the need to worry about trailing slashes altogether.

@dandibot
Copy link
Member

dandibot commented Aug 3, 2022

🚀 Issue was released in v0.2.42 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Aug 3, 2022
@jwodder
Copy link
Member

jwodder commented Jun 8, 2023

@waxlamp @AlmightyYakob Was this PR reverted/undone at some point? Currently, making a request to a Dockerized API instance at http://localhost:8000/server-info, with or without a trailing slash, returns 404.

@jjnesbitt
Copy link
Member

@waxlamp @AlmightyYakob Was this PR reverted/undone at some point? Currently, making a request to a Dockerized API instance at http://localhost:8000/server-info, with or without a trailing slash, returns 404.

/server-info is a route on the dandi GUI, which redirects to the API /info endpoint. So if you're making requests to the API directly you need to use the /info endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants