-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[WIP] Support collections as 'single item' in Libraries #11806
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
base: dev
Are you sure you want to change the base?
[WIP] Support collections as 'single item' in Libraries #11806
Conversation
For naming consistency.
And modify LibraryFolder model to support collections.
This will create and use a LDCA in the library instead of `extracting` all the datasets in the library. IMPORTANT: permissions are not managed yet.
Adds the library_dataset_collection table to the database
LDCAs were only ever sketched out - not really implemented so this is really interesting and I had no idea it was coming. You said:
Is there a reason not to just use the LDCAs right in the library? What metadata belongs on LDCs and what metadata belongs on LDCA in your mind? This is fascinating and exciting and a bit terrifying. So nothing in the models really ensures that LDCAs only contain library datasets and HDCAs only contain history datasets and in fact the first issue I was given said they should be interchangeable that way - I've really come to realize that is not a viable user experience though. I would suggest implementing checks and such to make sure library collections only contain library datasets and history collections only contain HDAs going forward. |
…em in libraries. Or else, use the default behavior of unpacking datasets.
Thanks, @jmchilton that is exactly the kind of feedback I need. I am trying to learn more about HDAs, HDCAs, LDDAs, LDCAs... right now everything still sounds a bit like a tongue-twister in my head 😆
That may explain why I had difficulties trying to build the main picture... 😅
Honestly, I just tried to get something half-working to set the ground for discussion and feedback while I try to get myself more familiar with these concepts. So I just tried to kind of mimic what is already there with LibraryDataset and LibraryDatasetDatasetAssociation but I still need to wrap my head around all the concepts.
I am stuck in the terrifying part 🤣 |
This is a very WIP, but I wanted to get some feedback to see if this is going in the right direction or I'm completely off-road here.
What did you do?
I basically tried to simulate what the Library API is doing right now with simple datasets, so I:
LibraryDatasetCollection
model to act as a proxy of an existing LDCA._copy_hdca_to_library_folder_single
method in the controller that will add a newLibraryDatasetCollection
to the database and return the basic information:folder_contents
list three types of items, folders, datasets, and collections.FolderContentsService
inputs and outputs, this probably should have been part of the API modernization in a different PR but they usually help me better understand what is being returned by the API. Anyway, now it will be really easy to just add the FastAPI endpoints :)Why did you make this change?
This is related to #11250 and somewhat a follow-up on #3559 but trying to add the collection as a single library item that will act as a special kind of folder, instead of importing all the contained datasets as individual items.
Open questions / Feedback required
LibraryDatasetPermissions
and also for associations?I have replaced the previous call in the API to add a new HDCA by unpacking the datasets with the one that returns the LDCA but I didn't remove the previous implementation. Maybe is a good idea to add an optional bool parameter to CreateLibraryItemPayload that will call the old way of importing collections by default so there is no change for existing calls to the API that expect a list of datasets as return value.Easier to do than to explain :) see 109264bHow to test the changes?
Datasets -> Import form History
License
For UI Components
Here is a preview of what is showing now without any changes in the client:
