Rework "Zarr multipart flag rework" - #2887
Conversation
mvandenburgh
left a comment
mvandenburgh
left a comment
There was a problem hiding this comment.
I agree this is cleaner. Overall looks great, just a few bugs and places that need to be updated.
| def abort(self) -> None: | ||
| super().abort() | ||
|
|
||
| # This upload owns its object key outright, so the object can be deleted along with it | ||
| self.blob.delete(save=False) | ||
|
|
There was a problem hiding this comment.
Things like this make me realize we never implemented a service layer for uploads. Obviously, that's completely out of scope for this change, but it might be worth revisiting at some point.
| def _has_active_uploads(ds: Dandiset) -> bool: | ||
| """Return whether the dandiset has any in-progress asset blob or zarr chunk uploads.""" | ||
| return ds.uploads.exists() or ZarrUpload.objects.filter(zarr__dandiset=ds).exists() |
There was a problem hiding this comment.
We should also include this logic here https://github.com/dandi/dandi-archive/blob/master/dandiapi/api/views/dandiset.py#L612-L648. The logic below prevents a dandiset with active zarr chunk uploads from being unembargoed on the backend, but the Unembargo button in the UI uses the uploads list endpoint to determine whether to allow clicking it.
There was a problem hiding this comment.
Along similar lines, there's also this https://github.com/dandi/dandi-archive/blob/master/dandiapi/api/models/version.py#L89-L91, which is used to populate the "active uploads" frontend view.
There was a problem hiding this comment.
Your second point is easily fixed via 0ec60be.
The first point however is a bit more intrusive, as now we're mixing two different models within one endpoint. However, we do still need to address it. I'll see what works out best, but we'll have to change that endpoint in some way.
9d61609 to
3be87c7
Compare
Zarr uploads must be garbage collected differently than regular uploads, as existing keys can be overwritten.
a8d4fd1 to
7c314fb
Compare
2 participants
This PR reworks #2869 to split up the zarr upload endpoints and models from that of asset blobs. This results in a much cleaner API design that is still backwards compatible with the CLI, as the existing single-part upload endpoints aren't removed.