Fix asset update/deletion when associated with zarr

[jjnesbitt]

Closes #1336

The reported issue was caused by the following:

• When a zarr is created and associated to an asset (but not ingested) it initially has a size of zero
• Ingest can then be performed, giving it a non-zero size
• If the containing asset is then updated / deleted, all parent paths are updated, decrementing the size of the asset. However, now that size is non-zero, and was never updated, so a possibly zero value is being decremented by a non-zero value, causing the value to be negative, and throwing an integrity error.

This PR updates the ingest_zarr_archive task to delete all asset paths that are associated with the zarr, and then re-add them after the ingestion is finished. This is essentially the same process as updating an asset path (path deletion and immediate addition), but with zarr ingestion in the middle.

[jjnesbitt]

It seems this is still failing for some reason. Investigating...

[jjnesbitt]

@jwodder I can no longer reproduce this error locally, using your MVCE. I was originally able to, but it seems that inexplicably (no local code changes), those now succeed. I've also reset my local DB to no avail. Yet, they still fail in CI.

Would #1339 fix this issue? Even the master tests fail, so it would seem the answer is no, but I'm not totally sure on the inner-workings of those tests.

Are you still able to reproduce this locally (with the changes from this branch)?

[jwodder]

@AlmightyYakob Testing locally:

• The MVCEs I posted fail with a 500 error when run against staging
• The dandi-cli tests fail when run against a locally-built image of dandi-archive's master branch and pass when using this branch

Note that the CLI integration tests here currently use the master branch of dandi-archive regardless of what PR is being tested. Once #1339 and dandi/dandi-cli#1141 are merged, the tests will go back to using the current PR.

[jjnesbitt]

@AlmightyYakob Testing locally:

• The MVCEs I posted fail with a 500 error when run against staging
• The dandi-cli tests fail when run against a locally-built image of dandi-archive's master branch and pass when using this branch

Note that the CLI integration tests here currently use the master branch of dandi-archive regardless of what PR is being tested. Once #1339 and dandi/dandi-cli#1141 are merged, the tests will go back to using the current PR.

I see, that makes sense now. Thanks!

[mvandenburgh]

One question about using .iterator(), but this LGTM. I do think that it's not ideal to have to delete and recreate the asset paths on zarr ingest, but this seems more like a flaw in the overall design of Zarr/how we kind of consider zarrs Assets, but not really in some places.

[yarikoptic]

This PR updates the ingest_zarr_archive task to delete all asset paths that are associated with the zarr, and then re-add them after the ingestion is finished.

are we talking about paths within zarr, ie thousands of them? do you have an estimate of runtime necessary to delete and add e.g. 200k paths?

[jjnesbitt]

are we talking about paths within zarr, ie thousands of them? do you have an estimate of runtime necessary to delete and add e.g. 200k paths?

No, we don't store the paths within zarr files. This is referring to all of the assets that point to zarrs. Zarrs have to be handled differently here, since zarr ingestion can update the asset size, yet subverts the normal asset update process.

[dandibot]

🚀 PR was released in v0.3.0 🚀