WIPACrepo / file_catalog

Store file metadata information in a file catalog
MIT License
1 stars 4 forks source link

[minor] Allow Non-Unique `logical_name`s #110

Closed ric-evans closed 2 years ago

ric-evans commented 2 years ago

from https://github.com/WIPACrepo/file_catalog/issues/109

also closes #113

ric-evans commented 2 years ago

@dsschult I didn't have the bulk of the added/refined logic when you last looked at this

ric-evans commented 2 years ago

I still need to write/fix tests

ric-evans commented 2 years ago

@dsschult @blinkdog - I distinguish two types of guardrails in the various REST methods: validation (static/stateless) and deconfliction (stateful). Validation checks the schema of the incoming data, whereas Deconfliction checks whether the incoming data will conflict with something already in the database.

ric-evans commented 2 years ago

Old tests are fixed and/or deleted (where logic changed drastically). New tests tomorrow.

ric-evans commented 2 years ago

These comments are nit-picking. If you think they aren't necessary changes, feel free to push back and I'll change it to Approve rather than Request changes.

it does help that we're nitpicky about similar things :smile:

ric-evans commented 2 years ago

we'll need to change the MongoDB logical_name index to unique=False in the active db

ric-evans commented 2 years ago

wait for https://github.com/WIPACrepo/wipac-telemetry-prototype/issues/30 before merge, then require the newest WIPACTEL version

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert and fixes 1 when merging 3a5679dbcdc644615ca5737320543ac279bfb1eb into e7879ca330d9fe80cc52454b776cc9246d415f46 - view on LGTM.com

new alerts:

fixed alerts:

ric-evans commented 2 years ago

A few more things need to happen before merge (in order):

  1. https://github.com/WIPACrepo/file-catalog-indexer/issues/31
  2. https://github.com/WIPACrepo/lta/issues/208
  3. https://github.com/WIPACrepo/file_catalog/issues/115 -- manually update (delete+build) indexes on the current MongoDB (as late as possible):
    • logical_name
    • locations.site-locations.path Compound Index

Edit: re-order Edit: add compound index step Edit: add issue for indexes

dsschult commented 2 years ago
* manually update the `logical_name` index on the current MongoDB

We should do this in the hours before the new version is pushed to prod, so the old version doesn't restart and try re-creating the old index. Note that I think this needs to be done manually because the index is named the same, and it won't know to delete and re-create the index.

ric-evans commented 2 years ago

@blinkdog @dsschult looking to merge this in on/after Oct 6. After the client scripts have been taken care of.

ric-evans commented 2 years ago

closes #113

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert and fixes 1 when merging a5ab74d475f3fe1b1355e1100830c3caab6ee03b into fe141d68c784fad75e1f8aae6a9187ba717e6c62 - view on LGTM.com

new alerts:

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert and fixes 1 when merging 9eb2c1f2d70c02edaf44c1c9c9960e57f0d72793 into fe141d68c784fad75e1f8aae6a9187ba717e6c62 - view on LGTM.com

new alerts:

fixed alerts: