simonsobs / librarian

The HERA Librarian.
BSD 2-Clause "Simplified" License
1 stars 2 forks source link

Gp/fix issue 54 #65

Closed iparask closed 5 months ago

iparask commented 6 months ago

@JBorrow, this PR introduces the instances API for deletion.

We can further abstract it by including an instance type in the models and have a single endpoint for both, as they depend on a session.

Also, some unit tests broke, and I am not sure if fixing some typos was the reason or the deletes. Do you have an idea?

closes #54

JBorrow commented 6 months ago

Can you add the instance IDs to the search_files endpoint too? Otherwise we can't figure out what instance ids there are!

JBorrow commented 6 months ago

Finally, for local instances, we probably want the ability to (or not) delete the underlying file associated with the instance.

iparask commented 6 months ago

... for local instances, we probably want the ability to (or not) delete the underlying file associated with the ins

We can check if the instance to delete is the only local instance the file is associated with and with no remote instances. In that can we can remove the file, otherwise it remains in the filesystem.

JBorrow commented 6 months ago

I actually think it's the opposite of that. You would want to delete the file on disk if there are remote instances. At the site we have limited storage space, and so we will naturally need to delete instances of files (though we want to maintain a complete list of all 'files' that have been ingested) once they appear in multiples outside the site.

iparask commented 5 months ago

It needs a couple of unit tests for instance-searches. But this is the proposed solution.

JBorrow commented 5 months ago

Fantastic, thank you for the contribution!