Skip missing history item ids in job exports#22563
Skip missing history item ids in job exports#22563guerler wants to merge 6 commits intogalaxyproject:devfrom
Conversation
|
That seems really very strange and I don't think we ca just silently ignore this. Can you reproduce this ? |
| model_store_format, | ||
| export_files=export_files, | ||
| user_context=user_context, | ||
| tolerate_missing_data=True, |
There was a problem hiding this comment.
The default is already False. Here it's set explicitly only for the archival path. If we set it to False here too, we can remove the flag entirely, since it won't be used anywhere, and then exports would continue to fail.
There was a problem hiding this comment.
How's that a flag ? You need to expose this in the API so the batch exporter can do a best-effort version of the export, but regular history exports should fail, not pretend they worked.
| assert options.get_identifier_for_id(id_encoder=None, obj_id=None) is None | ||
| assert options.get_identifier_for_id(id_encoder=None, obj_id=0) == 0 | ||
| assert options.get_identifier_for_id(id_encoder=None, obj_id=42) == 42 | ||
| def test_get_identifier_for_id_null_handling(): |
There was a problem hiding this comment.
I don't think this test proves anything but the literal change you made doesn't crash. Nothing there proves downstream work actually works.
| def test_get_identifier_for_id_null_handling(): | ||
| """Null job-param ids raise in strict mode and pass through in | ||
| tolerate_missing_data mode.""" | ||
| def test_job_serialize_with_null_param_ids(): |
There was a problem hiding this comment.
We have a ton of existing model store unit tests. Use one of those and delete the relevant item before export.
| * @description Best-effort export: skip unresolved references instead of failing. | ||
| * @default false | ||
| */ | ||
| tolerate_missing_data: boolean; |
There was a problem hiding this comment.
it isn't missing data though, it's missing provenance. The job is very old, i think it probably predates collections. i would just call it ignore_errors and document that this is a last resort, exported data may be corrupt.
github-actions
Bot
changed the title
|
Switched to targeting dev, since this seems to only impact |
guerler
force-pushed
the
fix_22562
branch
3 times, most recently
from
a46ee16 to
73c7c77
Compare
2 participants
Fixes #22562. Branched from: #22561.
How to test the changes?
(Select all options that apply)
License