Skip to content
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

ArtifactDetection creates orphan IntervalLists #1194

Open
samuelbray32 opened this issue Nov 25, 2024 · 2 comments · May be fixed by #1195
Open

ArtifactDetection creates orphan IntervalLists #1194

samuelbray32 opened this issue Nov 25, 2024 · 2 comments · May be fixed by #1195
Labels
bug Something isn't working common

Comments

@samuelbray32
Copy link
Collaborator

Describe the bug

  • ArtifactDetection (v0 and v1) creates entries in IntervalList
    • ArtifactDetection does not have a foreign key reference to IntervalList
    • These entries are then orphans at the time of creation
    • Eventually are de-orphaned if the user uses them in a spikesorting/etc.
  • SpyglassMixin.delete() cleans up orphans in IntervalList
    • If a second user executes this between the time an artifact interval list is created by spikesorting.v1.ArtifactDetection and when it is referenced elsewhere, these intervals will be deleted
    • Creates entries in ArtifactDetection with no corresponding intervals
    • Creates user confusion in spikesorting on an active database

Solution Options

  1. Add foreign key reference to IntervalList to the ArtifactDetection tables
    • Cleanest solution
    • Not sure if feasible to alter tables in this way
  2. Less aggressive cleanup of interval orphans
    • Rather than on every user delete call, clean up orphans in the nightly cleanup
    • Gives time for users to reference these entries elsewhere
  3. Dummy table to de-orphan these
    • Make an AdoptionTable downstream of IntervalList that serves only to reference keys in there
    • Could add an option IntervalList.insert1(key, adoption=True) to be used by artifact detection inserts that would automatically put these entries in the AdoptionTable as well
    • Would prevent them from being cleaned up immediately by deletes
    • Could log creation times of entries in AdoptionTable and have a cleanup policy there (e.g. only are protected in this table for 1 week before deletion)

@CBroz1, happy to implement this, but does any option sound preferable to you?

@samuelbray32 samuelbray32 added bug Something isn't working common labels Nov 25, 2024
@samuelbray32
Copy link
Collaborator Author

Affected Entries

  • v1: 24
  • v0: 12

@CBroz1
Copy link
Member

CBroz1 commented Nov 26, 2024

I agree that adding a fk ref would be the cleanest solution. This has been on DJ's roadmap for some time (#901). It is possible in SQL, but we'd have to be careful about following DJ's conventions for fk refs, as well as making it nullable (-> [NULLABLE] TableName)

I think the safer option is to move the orphan cleanup to the nightly cleanup routine

@samuelbray32 samuelbray32 linked a pull request Nov 26, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working common
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants