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

RF move redirector testing into dandiapi testing? #977

Open
yarikoptic opened this issue Apr 21, 2022 · 7 comments · May be fixed by #1020
Open

RF move redirector testing into dandiapi testing? #977

yarikoptic opened this issue Apr 21, 2022 · 7 comments · May be fixed by #1020
Assignees
@yarikoptic-gitmate
Labels
blocked Blocked by some needed development/fix

Comments

@yarikoptic
Copy link
Member

follow up (might be contingent on) to #976 , noticed that it is the only test aiming for redirector testing

$> git grep -2 mark.redirector
dandi/tests/test_utils.py-
dandi/tests/test_utils.py-
dandi/tests/test_utils.py:@pytest.mark.redirector
dandi/tests/test_utils.py-@using_docker
dandi/tests/test_utils.py-def test_server_info() -> None:

and it is not ran as part of the dandi/dandi-archive#1054 as it now should since it would be the one to provide server-info
@jwodder
Copy link
Member

jwodder commented Apr 21, 2022

@yarikoptic The --dandi-api option used by the dandi-archive + dandi-cli integration tests only selects tests that use the Docker setup, and "adjusting" the test by just setting DANDI_REDIRECTOR_BASE in the integration tests will likely mess up various other tests. Should a separate test be added that does the same thing as this one, only focused on the Docker setup, or should the test be rewritten completely to only ever test against Docker? Or something else?

@jwodder
Copy link
Member

jwodder commented May 3, 2022

@yarikoptic Ping; please respond to previous comment.

@yarikoptic
Copy link
Member Author

Proceed with the minimal impact. I guess testing against dandi-archive docker setup (and thus "development" version in their CI iirc) would be sufficient.

@jwodder
Copy link
Member

jwodder commented May 12, 2022

@yarikoptic What exactly are you asking for?

@yarikoptic
Copy link
Member Author

since redirector service was deprecated and now dandi-archive is the one providing /server-info that test should be adjusted so that testing in dandi-archive

(git)lena:~/proj/dandi/dandi-archive[enh-release]git
$> git grep -3 pytest .github
.github/workflows/cli-integration.yml-
.github/workflows/cli-integration.yml-      - name: Run dandi-api tests in dandi-cli
.github/workflows/cli-integration.yml-        run: |
.github/workflows/cli-integration.yml:          python -m pytest -s -v --dandi-api \
.github/workflows/cli-integration.yml-            "$pythonLocation/lib/python${{ matrix.python }}/site-packages/dandi"
.github/workflows/cli-integration.yml-        env:
.github/workflows/cli-integration.yml-          DANDI_TESTS_PERSIST_DOCKER_COMPOSE: "1"

would test its new version/PR against it.

@jwodder
Copy link
Member

jwodder commented May 26, 2022 •
edited
Loading

@yarikoptic Are you saying that the Dockerized Dandi Archive now provides a /server-info endpoint? Because, after updating the test, I'm getting a 404.

This seems to imply that /server-info is a redirect to /api/info/ provided by netlify.

@yarikoptic yarikoptic mentioned this issue May 26, 2022
Closed
@yarikoptic
Copy link
Member Author

you must be correct and I was wrong/mislead -- most likely indeed redirects since I see no mentioning of server-info in datalad-archive... filed dandi/dandi-archive#1108

@yarikoptic yarikoptic added the blocked Blocked by some needed development/fix label May 26, 2022
@jwodder jwodder linked a pull request May 26, 2022 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yarikoptic-gitmate yarikoptic-gitmate

Labels
blocked Blocked by some needed development/fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Test /server-info endpoint in Dockerized environment dandi/dandi-cli
3 participants
@yarikoptic @jwodder @yarikoptic-gitmate