Discussion re extending download to support multiple blob files per resource

rufuspollock commented 3 years ago

# bit more complex as have verify and multipart
def get_upload_url_and_headers(file_id=sha256, file_size, org/dataset, my-identity) => 
  {
    upload_url,
    headers
  }

def get_download_url_and_headers(file_id=file_sha256, file_size, org/dataset, my-identity) => {download_url}

Storage bucket:

{bucket}/{configured prefix}/{lfs_prefix := org/dataset}/{file-path usually sha256}

Uploading files:

Get an authz token from CKAN
Go to giftless with that token
Exchange that token for an upload token
Upload

Downloading files ...

Have a desired url (?)
Get an authz token from CKAN
Go to giftless with that token + desired url
Exchange that token for download token (ie. url + headers)
Download the file ...

How CKAN (ckanext-blob-storage) works (?):

url of a resource: https://ckan.nhs.staging.datopian.com/dataset/65050ec0-5abd-48ce-989d-defc08ed837e/resource/26f3d260-9b90-40c8-90de-c540704f59ac/download/epd_201403
that url is handled somewhere and then redirects
Qu: why have 2 levels of indirection

Questions:

Should we assume everything in the storage is always non-public and you needed signed credentials to get in? YES, this is the secure way to do things
- Atm you can't set this per resource in your call to Giftless.
What is stored onto a resource?
- What is added to resource_show / package_show? Preferenece would be to have a url with time limited expiry ... No, that does not work as they have their browswer open for 30m and then does not work
How do i set download name for a file as part of upload? (can i set a header)? or is that set on downloading?
- Can do in upload and download. We do in both.
- On Azure: set content-disposition header on upload and download
- On GCS: set content-disposition header on download: https://github.com/datopian/giftless/blob/466df8c071aa1e5702bcc28c3b153258e9f3da48/giftless/storage/google_cloud.py#L92. Don't set on upload ... (TODO: ...)
How would giftless work without CKAN e.g. integrated with git? Specifically, how would one do authorization ..

Proposal

Proposal B:

Modify download handler to be a bit more generic:

{ckan-instance}/dataset/65050ec0-5abd-48ce-989d-defc08ed837e/resource/26f3d260-9b90-40c8-90de-c540704f59ac/download/sha256:{sha256::size}

  => 404, 401 or 302 to download location

Proposal A: download_url API - rejected because we don't want an API ...

resource:

{
  // current
  url: {ckan-instance}/dataset/65050ec0-5abd-48ce-989d-defc08ed837e/resource/26f3d260-9b90-40c8-90de-c540704f59ac/download/{file-name}
  // new
  url: {ckan-instance}/api/3/action/download_url?sha256=...,size=...,dataset=...,resource=...
  zip_url: {ckan-instance}/api/3/action/download_url?...
}

What I want ...

is the ability to store more than one piece of blob data for a resource and get download urls for that

shevron commented 3 years ago

I think the "multiple files per resource" think is, to the best of my knowledge, not something that is cleanly supported by CKAN. I realize we have some customisations in other projects that allow this, but I'm wondering if is something that should be generically supported by this extension.

I think option B is easy to implement and if we want to support it generically, could be the way to go.

Another option I can suggestion, which is what I consider a cleaner variation of option B, is this:

ckanext-blob-storage is slightly refactored to offer Python API (e.g. a helper function or similar) that allows downloading anything stored on Giftless as long as we have the organization, dataset, expected sha256 and size of file. It will do the work of authorizing the current user and getting the download link from Giftless, and return it or raise an exception (e.g. if not found or unauthorized). The current download blueprint will be refactoed to use this. Actually, this is not far from what we have right now.
Any projects that want to specifically offer multiple files per resource will implement their own CKAN blueprint / endpoint in their own CKAN extension using this helper function, with a cleaner URL. They can choose to override the current download URL or not.

This will allow ckanext-blob-storage to remain clean and not have code for these kind of special cases.

rufuspollock commented 3 years ago

@shevron agree with you re Option B being the way to go. Also agree that i think it can be changed pretty easily.

I do think it would be worth generifying the default endpoint right now but that's something we can discuss.

How much work do you think it is to do this?

shevron commented 3 years ago

Probably ~1 day but hard to fully estimate without spending some time on analysis. I don't think there is complexity here, just figuring out the right API.

shevron commented 3 years ago

To update, in the latest merge I actually refactored the action that provides download URL / headers to wrap a factored-out Python function (https://github.com/datopian/ckanext-blob-storage/blob/master/ckanext/external_storage/actions.py#L28) that now allows specifying a different sha256 / size / filename for the given resource; This change will allow this extension or other extensions to easily implement the approach I suggested.

datopian / ckanext-blob-storage