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

ENH: list localhost:8085 as the web UI for dandi-api-local-docker-tests #1003

Merged
merged 4 commits into from
Aug 4, 2022
Merged

ENH: list localhost:8085 as the web UI for dandi-api-local-docker-tests #1003

merged 4 commits into from
Aug 4, 2022

Conversation

yarikoptic
Copy link
Member

Troubleshooting dandi-archive locally and needed to download a new
dandiset from it. But it was not working because of

$> dandi -l DEBUG download -f debug http://localhost:8085/dandiset/000002
2022-05-05 16:46:10,698 [   DEBUG] Starting new HTTPS connection (1): rig.mit.edu:443
2022-05-05 16:46:10,961 [   DEBUG] https://rig.mit.edu:443 "GET /et/projects/dandi/dandi-cli HTTP/1.1" 200 579
2022-05-05 16:46:10,962 [   DEBUG] Running a newer version (0.39.5+4.gcca19df4.dirty) of dandi/dandi-cli than available (0.39.5)
2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 7 to 5
2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 5 to 7
2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 7 to 5
2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 5 to 7
2022-05-05 16:46:12,556 [   DEBUG] Parsing url http://localhost:8085/dandiset/000002
2022-05-05 16:46:12,611 [   DEBUG] Parsed into DandisetURL(api_url=AnyHttpUrl('http://localhost:8085', scheme='http', host='localhost', host_type='int_domain', port='8085'), dandiset_id='000002', version_id=None)
2022-05-05 16:46:12,613 [   DEBUG] Initializing mode with given value of None
2022-05-05 16:46:12,614 [   DEBUG] Setting write mode to 'update'
2022-05-05 16:46:12,614 [   DEBUG] Setting width to stream width: 213
2022-05-05 16:46:12,617 [   DEBUG] GET http://localhost:8085/dandisets/000002/
2022-05-05 16:46:12,622 [   DEBUG] Starting new HTTP connection (1): localhost:8085
2022-05-05 16:46:12,637 [   DEBUG] http://localhost:8085 "GET /dandisets/000002/ HTTP/1.1" 404 156
2022-05-05 16:46:12,638 [   DEBUG] Response: 404
2022-05-05 16:46:12,638 [   ERROR] Error 404 while sending GET request to http://localhost:8085/dandisets/000002/: <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /dandisets/000002/</pre>
</body>
</html>

2022-05-05 16:46:12,638 [   DEBUG] Caught exception No such Dandiset: '000002'
2022-05-05 16:46:12,638 [    INFO] Logs saved in /home/yoh/.cache/dandi-cli/log/20220505204610Z-120654.log
Error: No such Dandiset: '000002'

There might be a better fix, since in general we know that our archive is one
page app, and thus requesting such urls directly would lead to such 404s, so we
should act accordingly. But meanwhile I have decided to just announce
availability of such an instance of an archive which is started locally from
dandi-archive repo. That seems to resolve the issue and allows to download
(and hopefully to upload, which will be tried shortly).

@yarikoptic yarikoptic requested a review from jwodder May 5, 2022 21:21
@codecov
Copy link

codecov bot commented May 9, 2022 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.47%. Comparing base (84f3ead) to head (4a56d1c).
Report is 956 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1003      +/-   ##
==========================================
- Coverage   88.53%   88.47%   -0.06%     
==========================================
  Files          73       74       +1     
  Lines        9295     9309      +14     
==========================================
+ Hits         8229     8236       +7     
- Misses       1066     1073       +7
Flag Coverage Δ
unittests 88.47% <100.00%> (-0.06%) ⬇️

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 added a commit that referenced this pull request Jul 26, 2022
@yarikoptic
ENH: list localhost:8085 as the web UI for dandi-api-local-docker-tests
54cd080 
It is not really for the tests but if local dandi-archive instance is
established following instructions. This follows @jwodder idea instead of
#1003 with more explicit dedicated instance
This was referenced Jul 26, 2022
Closed
Closed
@yarikoptic yarikoptic changed the title BF: add "dandi-archive-local-docker" instance ENH: list localhost:8085 as the web UI for dandi-api-local-docker-tests Jul 26, 2022
@yarikoptic yarikoptic force-pushed the bf-local-doker-instance branch from 938c6ba to 3d84714 Compare July 26, 2022 21:35
@yarikoptic
BF: add "dandi-archive-local-docker" instance
c628f79 
Troubleshooting dandi-archive locally and needed to download a new
dandiset from it. But it was not working because of

	$> dandi -l DEBUG download -f debug http://localhost:8085/dandiset/000002
	2022-05-05 16:46:10,698 [   DEBUG] Starting new HTTPS connection (1): rig.mit.edu:443
	2022-05-05 16:46:10,961 [   DEBUG] https://rig.mit.edu:443 "GET /et/projects/dandi/dandi-cli HTTP/1.1" 200 579
	2022-05-05 16:46:10,962 [   DEBUG] Running a newer version (0.39.5+4.gcca19df4.dirty) of dandi/dandi-cli than available (0.39.5)
	2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 7 to 5
	2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 5 to 7
	2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 7 to 5
	2022-05-05 16:46:11,143 [   DEBUG] Creating converter from 5 to 7
	2022-05-05 16:46:12,556 [   DEBUG] Parsing url http://localhost:8085/dandiset/000002
	2022-05-05 16:46:12,611 [   DEBUG] Parsed into DandisetURL(api_url=AnyHttpUrl('http://localhost:8085', scheme='http', host='localhost', host_type='int_domain', port='8085'), dandiset_id='000002', version_id=None)
	2022-05-05 16:46:12,613 [   DEBUG] Initializing mode with given value of None
	2022-05-05 16:46:12,614 [   DEBUG] Setting write mode to 'update'
	2022-05-05 16:46:12,614 [   DEBUG] Setting width to stream width: 213
	2022-05-05 16:46:12,617 [   DEBUG] GET http://localhost:8085/dandisets/000002/
	2022-05-05 16:46:12,622 [   DEBUG] Starting new HTTP connection (1): localhost:8085
	2022-05-05 16:46:12,637 [   DEBUG] http://localhost:8085 "GET /dandisets/000002/ HTTP/1.1" 404 156
	2022-05-05 16:46:12,638 [   DEBUG] Response: 404
	2022-05-05 16:46:12,638 [   ERROR] Error 404 while sending GET request to http://localhost:8085/dandisets/000002/: <!DOCTYPE html>
	<html lang="en">
	<head>
	<meta charset="utf-8">
	<title>Error</title>
	</head>
	<body>
	<pre>Cannot GET /dandisets/000002/</pre>
	</body>
	</html>

	2022-05-05 16:46:12,638 [   DEBUG] Caught exception No such Dandiset: '000002'
	2022-05-05 16:46:12,638 [    INFO] Logs saved in /home/yoh/.cache/dandi-cli/log/20220505204610Z-120654.log
	Error: No such Dandiset: '000002'

There might be a better fix, since in general we know that our archive is one
page app, and thus requesting such urls directly would lead to such 404s, so we
should act accordingly.  But meanwhile I have decided to just announce
availability of such an instance of an archive which is started locally from
dandi-archive repo.  That seems to resolve the issue and allows to download
(and hopefully to upload, which will be tried shortly).
@yarikoptic
RF: fold having GUI into dandi-api-local-docker-tests instance
1d6aa3e 
Per @jwodder comment and thus to avoid ambiguity in reverse mapping
@yarikoptic yarikoptic force-pushed the bf-local-doker-instance branch from 3d84714 to 3d42e50 Compare August 3, 2022 13:54
@yarikoptic yarikoptic force-pushed the bf-local-doker-instance branch from 3d42e50 to 58ec075 Compare August 3, 2022 18:14
@yarikoptic
Copy link
Member Author

@jwodder - how to figure out why docker-compose stopped working in this PR? see e.g.
https://github.com/dandi/dandi-cli/runs/7658296726?check_suite_focus=true

dandi/tests/fixtures.py:260: in docker_compose_setup
    run(
/opt/hostedtoolcache/Python/3.10.5/x64/lib/python3.10/subprocess.py:524: in run
    raise CalledProcessError(retcode, process.args,
E   subprocess.CalledProcessError: Command '['docker-compose', 'run', '--rm', 'django', './manage.py', 'migrate']' returned non-zero exit status 1.

i.e could we see stderr?

@jwodder
Copy link
Member

jwodder commented Aug 3, 2022

@yarikoptic I believed I identified & reported the issue in dandi/dandi-archive#1230.

@yarikoptic yarikoptic force-pushed the bf-local-doker-instance branch from 58ec075 to 4a56d1c Compare August 3, 2022 21:29
This was referenced Aug 3, 2022
Closed
Merged
@yarikoptic
Copy link
Member Author

hm, windows failed with

________________ test_download_000027_resume[draft-<lambda>0] _________________
dandi\tests\test_download.py:131: in test_download_000027_resume
    assert sorted(contents) == ["sub-RAT123", op.join("sub-RAT123", "sub-RAT123.nwb")]
E   AssertionError: assert ['sub-RAT123'...wnload\\lock'] == ['sub-RAT123'...b-RAT123.nwb']
E     At index 1 diff: 'sub-RAT123\\sub-RAT123.nwb.dandidownload' != 'sub-RAT123\\sub-RAT123.nwb'
E     Left contains 3 more items, first extra item: 'sub-RAT123\\sub-RAT123.nwb.dandidownload\\checksum'
E     Full diff:
E       [
E        'sub-RAT123',
E     -  'sub-RAT123\\sub-RAT123.nwb',
E     +  'sub-RAT123\\sub-RAT123.nwb.dandidownload',
E     ?                             ++++++++++++++
E     +  'sub-RAT123\\sub-RAT123.nwb.dandidownload\\checksum',
E     +  'sub-RAT123\\sub-RAT123.nwb.dandidownload\\file',
E     +  'sub-RAT123\\sub-RAT123.nwb.dandidownload\\lock',
E       ]

I will just restart it...

@yarikoptic yarikoptic requested a review from jwodder August 3, 2022 22:09
jwodder
jwodder requested changes Aug 4, 2022
View reviewed changes
dandi/cli/tests/test_download.py
@pytest.mark.skipif(
bool(known_instances["dandi-api-local-docker-tests"].gui),
reason="this instance now has GUI URL",
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change the URL in the test to http://localhost:8085/#/dandiset/123456/draft, the test will pass and there won't be a need for the skipif.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;-) that is what I pretty much did first but then looked at the name of the test : test_download_api_instance_in_dandiset. And then (copy pasted?) test above differing only in name test_download_gui_instance_in_dandiset and url to point to GUI. So, it is that test above which already checks for that code path and this one was intended to check that we would resolve to api instance if there is no GUI url. As such, there is no need for two identical tests, and I decided to skip it while keeping for future happen we kill gui there again or smth like that

@yarikoptic yarikoptic merged commit dc69353 into master Aug 4, 2022
@yarikoptic yarikoptic deleted the bf-local-doker-instance branch August 4, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwodder jwodder jwodder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yarikoptic @jwodder