pcdshub / happi

Heuristic Access to Positions of Photon Instruments
https://pcdshub.github.io/happi/master
Other
13 stars 29 forks source link

ENH: Match Name to ID Field #327

Closed laura-king closed 1 year ago

laura-king commented 1 year ago

Description

When running happy repair, will check if name and id fields are matching. If not, will set the name field to the id field.

Added comparison and attribute setting in repair function. Added click context-based unit test for setting name and id equivalence.

Changed 'res_name' variable to 'res_id' to represent correct value.

Motivation and Context

Closes #302

How Has This Been Tested?

Unit test using click contexts that call repair on the bad-happi-config.

Where Has This Been Documented?

Here

Pre-merge checklist

tangkong commented 1 year ago

The main caveat there (I think) is that a working db will have no conflicting id's, but could have conflicting names. So setting id <- name might cause a collision. (In this case if the _id is duplicated, only one will appear in happi searches, etc)

But Ken's point is still true, the opposite (id -> name) gives us less control

tangkong commented 1 year ago

Ah I also just noticed you pushed to the pcdshub repo instead of your own fork. We typically abide by the fork-branch-pullrequest model, as it keeps our repos clean and works better with some of our CI jobs.

It's ok this time, but next time you should make changes to a branch on your own fork, then submit a pull request to merge that fork (e.g. laura-king:repair-name) into pcdshub:master

ZLLentz commented 1 year ago

I realized that in the case where there's two items with the same _id, one of them will just get nuked by this.

I don't think this is possible in the json backend, unless you hand-edit the files

klauer commented 1 year ago

I don't think this is possible in the json backend, unless you hand-edit the files

Right, because the JSON looks like this:

"_id_name": {
  "_id": "_id_name",
  "name": "ophyd_name"
},

Making up an _id of "_id_name", the key of the item dictionary has to match the "_id" entry in the key. "name" can be different, but we force it to be the same as in this PR.