-
Notifications
You must be signed in to change notification settings - Fork 1
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
Do store/expose logs for (at least failed) runs. #72
Comments
ping on this |
@rly please sign up to this issue to know when you can get to the logs seamlessly |
Are you sure that's necessary? I expect the repository will end up containing a large number of small files either way, so most of the space consumption will be due to the amount of files rather than their contents. Moreover, seeing as we're currently having performance issues with just testing one asset per Dandiset, and assets that haven't been tested recently are preferred, I expect that individual files in the repository will change infrequently, so rewriting history still won't save much space.
|
@yarikoptic Also, if we move all of |
@yarikoptic Ping. |
|
|
@yarikoptic Ping. |
1 similar comment
@yarikoptic Ping. |
Let's store for all for starters -- might be worth comparing between successful and failing runs
I feel we better separate.
could we use some field as
no, keep it here
I guess so |
There's no single field in the issue 82 records (yet) that uniquely identifies each record, so adding a unique ID field (whether UUID or some other format) is the only option.
Why the name change from https://github.com/dandi/dandisets-healthstatus-logs?
I'm not a fan of that file path format. I'd prefer sticking to the initial idea of |
@yarikoptic Ping on responses to above. |
Ok, let's keep logs/ and https://github.com/dandi/dandisets-healthstatus-logs and your path format. |
@yarikoptic So, to summarize:
Is that correct & complete? |
Uff, starts to become tricky to keep track of discussion(s) in multiple issues etc. Next time we better do it as some design doc PR, since then it is easier to polish particular aspect(s) with Using convention of
The behaviors you described above otherwise I think are good. |
@yarikoptic See #88 for my thoughts on multiple repositories.
The test ID still needs to be stored somewhere. This was why I suggested storing all three values (test ID, stdout, and stderr) in a single YAML file. |
eh, if we didn't care about number of files/inodes, then we could add one more file, but indeed probably let's then just have a yaml |
In #82 (comment), you requested that an "untested" event be emitted whenever a new asset was found in a Dandiset. Is there supposed to be only one such event per asset — in which case, how would that fit in with the above path format? — or do you want one event for each test? There's also the fact that non-NWB assets are currently "tested" by running |
be kinda lame to populate with multiple "untested" events. |
If you're asking about the "untested" pseudo-test run on non-NWB assets, I don't see a problem with logging the If you're instead asking about the "untested" status for new assets, both NWB and non-NWB (which should really be renamed to "new", possibly alongside renaming "untested"), one approach would be to log them to a Dandiset-wide record file at |
my concern was more of the latter. Indeed probably would not make sense for |
Prior ref:
So we do store the logs from each run combined across all assets, e.g.
so to access a fail for some sample failing healthcheck, someone needs to
which is quite cumbersome. So, ideally we should have them committed to git and pushed directly here on github. The problem is that we can't keep history growing since then repo would grow too large too quickly. But I think we will be ok if we keep only the single state commit and rewrite it. So let's
logs/
submodule (https://github.com/dandi/dandisets-healthstatus-logs repo, created now)sub-YutaMouse57/sub-YutaMouse57_ses-YutaMouse57-161014_behavior+ecephys.nwb/
matnwb_nwbRead_errors.txt
logs/
--ammend'ing prior commit, or pretty much can recreate that branch from scratch every time, just reusing the tree of prior statelogs/
In the README.md here, per each dandiset row, add there a convenience link to the logs, so row looks like
000019 [logs] | 30 passed, 1 failed, 0 timed out | 0 passed, 31 failed, 0 timed out | —
Ideally, if it is feasible, if we could as a comment in
results/
yaml files urls to logs, likethat would have been great, but I have not looked how feasible it is yet.
The text was updated successfully, but these errors were encountered: