Test /server-info endpoint in Dockerized environment #1020

jwodder · 2022-05-26T18:31:38Z

Closes #977.

codecov · 2022-05-26T18:36:20Z

Codecov Report

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

Project coverage is 62.78%. Comparing base (94a426c) to head (5afe8e4).

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

❗ There is a different number of reports uploaded between BASE (94a426c) and HEAD (5afe8e4). Click for more details.

HEAD has 9 uploads less than BASE

Flag BASE (94a426c) HEAD (5afe8e4)

unittests 19 10

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1020       +/-   ##
===========================================
- Coverage   88.59%   62.78%   -25.82%     
===========================================
  Files          78       78               
  Lines       10855    10858        +3     
===========================================
- Hits         9617     6817     -2800     
- Misses       1238     4041     +2803

Flag	Coverage Δ
unittests	`62.78% <57.14%> (-25.82%)`	⬇️

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.

yarikoptic · 2022-07-28T19:27:56Z

dandi/dandi-archive#1108 was addressed/closed -- should we retry this PR now @jwodder?

jwodder · 2022-07-28T19:46:11Z

@yarikoptic I'm still getting a 404 for http://localhost:8000/server-info. dandi/dandi-archive#1190 indicates that only /server-info/ (with a trailing slash) is currently accepted, yet the code relies on accessing the path without a trailing slash.

yarikoptic · 2022-07-29T15:31:11Z

note: I have followed up on dandi/dandi-archive#1108 (comment) .

I think that the path forward of the least resistance would be to switch to use /server-info/ with a trailing slash.

both main and staging support that

$> curl https://dandiarchive.org/server-info/
{"schema_version":"0.6.3","schema_url":"https://raw.githubusercontent.com/dandi/schema/master/releases/0.6.3/dandiset.json","version":"0.2.41","services":{"api":{"url":"https://api.dandiarchive.org/api"},"webui":{"url":"https://dandiarchive.org"},"jupyterhub":{"url":"https://hub.dandiarchive.org/"}},"cli-minimal-version":"0.14.2","cli-bad-versions":[]}%  

$> curl https://gui-staging.dandiarchive.org/server-info/
{"schema_version":"0.6.3","schema_url":"https://raw.githubusercontent.com/dandi/schema/master/releases/0.6.3/dandiset.json","version":"0.2.41","services":{"api":{"url":"https://api-staging.dandiarchive.org/api"},"webui":{"url":"https://gui-staging.dandiarchive.org"},"jupyterhub":{"url":"https://hub.dandiarchive.org/"}},"cli-minimal-version":"0.14.2","cli-bad-versions":[]}%

or do you see any problem with that @jwodder ?

jwodder · 2022-07-29T15:32:35Z

@yarikoptic Would the Archive continue to support older versions of the CLI that use /server-info without a trailing slash?

yarikoptic · 2022-07-29T17:01:49Z

Well, ATM the actual deployment still has /server-info working through netlify somehow I guess. So unless and until that is changed, it would. Whenever it would stop we might need to boost minimal supported client version. For now I think it would be all good regardless.

yarikoptic · 2023-05-24T15:45:39Z

wouldn't this be made obsolete with #1298 @jwodder ?

jwodder · 2023-05-24T15:49:26Z

@yarikoptic No, get_instance() still makes requests to /server-info.

yarikoptic · 2023-06-07T22:51:19Z

@jwodder - please reapproach this PR to be finalized. It seems that trailing / is no longer needed:

❯ curl --silent https://dandiarchive.org/server-info
{"schema_version":"0.6.4","schema_url":"https://raw.githubusercontent.com/dandi/schema/master/releases/0.6.4/dandiset.json","version":"0.3.40","services":{"api":{"url":"https://api.dandiarchive.org/api"},"webui":{"url":"https://dandiarchive.org"},"jupyterhub":{"url":"https://hub.dandiarchive.org/"}},"cli-minimal-version":"0.51.0","cli-bad-versions":[]}

jwodder · 2023-06-08T12:39:42Z

@yarikoptic The Dockerized API instance currently returns 404 for http://localhost:8000/server-info with & without a trailing slash.

yarikoptic · 2023-06-08T13:23:19Z

@yarikoptic The Dockerized API instance currently returns 404 for http://localhost:8000/server-info with & without a trailing slash.

Please investigate why , check it with @AlmightyYakob et al.

yarikoptic · 2025-02-13T22:04:46Z

Resolved merge conflict, to see if we still experience the issue or this PR could be finalized.

yarikoptic · 2025-02-13T22:05:19Z

merge might need to be redone or rebased to address

Error: dandi/tests/test_utils.py:9:1: F811 redefinition of unused 'Iterable' from line 3
Error: dandi/tests/test_utils.py:9:1: F401 'typing.List' imported but unused

dandi/tests/test_utils.py

@@ -6,13 +6,15 @@
 import os.path as op
 from pathlib import Path
 import time
+from typing import Iterable, List


To fix the problem, we need to remove the unused import statement for List from the typing module. This will clean up the code and remove the unnecessary dependency. The change should be made in the dandi/tests/test_utils.py file, specifically on line 9.

yarikoptic · 2025-02-14T14:44:08Z

so it is still the case that for local instance it likely requires that trailing /:

_______________________________ test_server_info _______________________________
dandi/tests/test_utils.py:405: in test_server_info
    r.raise_for_status()
/opt/hostedtoolcache/Python/3.13.2/x64/lib/python3.13/site-packages/requests/models.py:1024: in raise_for_status
    raise HTTPError(http_error_msg, response=self)
E   requests.exceptions.HTTPError: 404 Client Error: Not Found for url: http://localhost:8000/server-info

looking at that point in browser gives more info:

and there is none with / neither. So, it is not a part of the django instance I guess, and served by smth else.

indeed, in dandi-archive

web/netlify.toml:package = "/netlify/plugins/server-info-build"
web/netlify/plugins/server-info-build/index.js:      from: '/server-info',
web/netlify/plugins/server-info-build/manifest.yml:name: server-info-build

so, it is netlify's thing and

❯ cat web/netlify/plugins/server-info-build/index.js
module.exports = {
  // Use onPostBuild hook so that the dist/ folder exists
  onPostBuild: ({ netlifyConfig }) => {
    let apiUrl = process.env.VITE_APP_DANDI_API_ROOT;
    if (apiUrl === undefined) {
      throw new Error('API URL not defined. Please define it with the VITE_APP_DANDI_API_ROOT environment variable.');
    }

    // Add redirect to server info
    apiUrl = apiUrl.endsWith('/') ? apiUrl.slice(0, -1) : apiUrl;
    netlifyConfig.redirects.unshift({
      from: '/server-info',
      to: `${apiUrl}/info/?format=json`,
      status: 200,
    });
  },
};

so just a redirect to http://localhost:8000/api/info/ which works

*(Pdb) r3 = requests.get(f"{root_url}/api/info/?format=json")
(Pdb) r3
<Response [200]>
*(Pdb) r3.body
*** AttributeError: 'Response' object has no attribute 'body'
*(Pdb) r3.content
b'{"schema_version":"0.6.9","schema_url":"https://raw.githubusercontent.com/dandi/schema/master/releases/0.6.9/dandiset.json","version":"0.4.18","services":{"api":{"url":"http://localhost:8000/api"},"webui":{"url":"http://localhost:8085"},"jupyterhub":{"url":"https://hub.dandiarchive.org"}},"cli-minimal-version":"0.60.0","cli-bad-versions":[]}'

@waxlamp @mvandenburgh -- why don't we just serve/redirect it directly from django?

jwodder added the tests Add or improve existing tests label May 26, 2022

yarikoptic added the blocked Blocked by some needed development/fix label Jun 7, 2022

jwodder force-pushed the gh-977 branch from ca532e3 to 8a3d88f Compare July 28, 2022 19:30

yarikoptic mentioned this pull request Jul 29, 2022

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

Closed

yarikoptic assigned jwodder Jun 7, 2023

Test /server-info endpoint in Dockerized environment

f75e916

jwodder force-pushed the gh-977 branch from 8a3d88f to f75e916 Compare June 8, 2023 12:40

yarikoptic-gitmate assigned yarikoptic-gitmate and unassigned jwodder Aug 10, 2023

Merge branch 'master' into gh-977

5afe8e4

github-advanced-security bot found potential problems Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test /server-info endpoint in Dockerized environment #1020

Test /server-info endpoint in Dockerized environment #1020

jwodder commented May 26, 2022

codecov bot commented May 26, 2022 •

edited

Loading

yarikoptic commented Jul 28, 2022

jwodder commented Jul 28, 2022

yarikoptic commented Jul 29, 2022

jwodder commented Jul 29, 2022

yarikoptic commented Jul 29, 2022

yarikoptic commented May 24, 2023

jwodder commented May 24, 2023

yarikoptic commented Jun 7, 2023

jwodder commented Jun 8, 2023

yarikoptic commented Jun 8, 2023

yarikoptic commented Feb 13, 2025

yarikoptic commented Feb 13, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

yarikoptic commented Feb 14, 2025

Test /server-info endpoint in Dockerized environment #1020

Are you sure you want to change the base?

Test /server-info endpoint in Dockerized environment #1020

Conversation

jwodder commented May 26, 2022

codecov bot commented May 26, 2022 • edited Loading

Codecov Report

yarikoptic commented Jul 28, 2022

jwodder commented Jul 28, 2022

yarikoptic commented Jul 29, 2022

jwodder commented Jul 29, 2022

yarikoptic commented Jul 29, 2022

yarikoptic commented May 24, 2023

jwodder commented May 24, 2023

yarikoptic commented Jun 7, 2023

jwodder commented Jun 8, 2023

yarikoptic commented Jun 8, 2023

yarikoptic commented Feb 13, 2025

yarikoptic commented Feb 13, 2025

yarikoptic commented Feb 14, 2025

codecov bot commented May 26, 2022 •

edited

Loading