Traverse assets via Archive API #81
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
==========================================
- Coverage 60.59% 60.50% -0.10%
==========================================
Files 10 11 +1
Lines 835 947 +112
Branches 191 212 +21
==========================================
+ Hits 506 573 +67
- Misses 309 351 +42
- Partials 20 23 +3 ☔ View full report in Codecov by Sentry. |
The key was added quite a while ago, and all files in `results/` have it now, so we don't need to keep supporting files that don't have it.
|
@yarikoptic FYI: I started a run with the new code in "random-outdated-asset-first" mode at 2:37 PM EDT. The script found all the Dandisets within two minutes. Currently, tests are being run on assets from Dandiset 000016 through 000036 (not all the ones in between, though), and 16 Dandisets have finished having their asset tested. |
|
I would check on drogon later on what it is busy with and either didn't stall again |
|
@yarikoptic I started another run yesterday at 4 PM, and it's still going now. I managed to parse out the following durations from the logfile for the tests that have completed so far: Interestingly, despite the timeout being set to 3600 seconds, a number of tests ran for longer than that (up to over two hours for some). I'm guessing that those just stalled after they were killed at the one hour mark. |
|
For me it is interesting that if I look at although generally it seems to log interesting interactions
so remains unclear what davfs2 is actually doing and why IO is actually quite slow (likely slower than our dataset fuse given that I do not even see python/matlab to appear in |
|
@yarikoptic Also, many of the failed MatNWB tests emitted the error: |
heh, hinting that may be even read outs could not be trusted, or it just times out internally... who knows (no source for matlab) . |
|
@yarikoptic The test run of this PR's code completed after 2 days and 7 hours. Since this PR has done what it set out to do and the remaining performance problems are unrelated to it, can this PR be merged now, with the remaining performance problems left for discussion in separate issues? |
This was referenced Jul 15, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in #77.
To do:
modifieddates fortest-filesDandisetinstances created fortest-filesdon't need a client (among other changes)