pcdshub / happi

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

Happi search didn't find all cxi devices #297

Closed ZLLentz closed 1 year ago

ZLLentz commented 1 year ago

Expected Behavior

happi search name=cxi* lightpath=False should show all cxi devices with lightpath=False

Current Behavior

It doesn't find them all (???)

Steps to Reproduce (for bugs)

  1. happi search name=cxi* lightpath=False
  2. note that kb1 is missing
  3. happi search cxi_kb1_hx
  4. note that lightpath=False here

Context

Trying to make cxi devices load again

Your Environment

pcds_conda 5.6.0

tangkong commented 1 year ago

Apparently the default in the container does not properly carry through to the item in a way that happi.Client.search_regex recognizes (possibly other search methods as well)

There's no concise way to describe this but: This script prints:

from happi.client import Client
from happi.client import SearchResult
cl = Client(path='/cds/home/r/roberttk/test/cxi_db_230210.json')

all_cxi_false = set([r.metadata['name'] for r in cl.search_regex(name='cxi.*', lightpath='False')])
all_cxi = set([r.metadata['name'] for r in cl.search_regex(name='cxi.*')])

print('cxi_kb1_hx' in all_cxi)
print('cxi_kb1_hx' in all_cxi_false)

True, True for:

{
  "_id": "cxi_kb1_hx",
  "active": true,
  "args": [
    "{{prefix}}"
  ],
  "beamline": "CXI",
  "creation": "Tue Aug 21 09:15:22 2018",
  "device_class": "pcdsdevices.device_types.IMS",
  "kwargs": {
    "name": "{{name}}"
  },
  "last_edit": "Tue Aug 21 09:23:32 2018",
  "lightpath": false,       <<<<<<<<<<<<<<
  "macros": null,
  "name": "cxi_kb1_hx",
  "parent": null,
  "prefix": "CXI:KB1:MMS:05",
  "screen": null,
  "stand": "KB1",
  "system": null,
  "type": "pcdsdevices.happi.containers.LCLSItem",
  "z": -1
}

True, False for:

{
  "_id": "cxi_kb1_hx",
  "active": true,
  "args": [
    "{{prefix}}"
  ],
  "beamline": "CXI",
  "creation": "Tue Aug 21 09:15:22 2018",
  "device_class": "pcdsdevices.device_types.IMS",
  "kwargs": {
    "name": "{{name}}"
  },
  "last_edit": "Tue Aug 21 09:23:32 2018",
  "macros": null,
  "name": "cxi_kb1_hx",
  "parent": null,
  "prefix": "CXI:KB1:MMS:05",
  "screen": null,
  "stand": "KB1",
  "system": null,
  "type": "pcdsdevices.happi.containers.LCLSItem",
  "z": -1
}
klauer commented 1 year ago

lightpath is missing in the cxi_kb1_hx dictionary, meaning that iterative_compare raises KeyError and the match is then thrown out.

Were these entries manually updated? Or does happi not write defaults to the JSON file on happi edit when a container changes?

A first-pass fix will be to update our database to include all required keys. Then we need to consider if happi should fill in missing keys from the container with their default values. I think the answer is "probably yes"?

https://github.com/pcdshub/happi/blob/7190c4feac94aea259af54e04bfbea58a43f6146/happi/backends/json_db.py#L259-L260

klauer commented 1 year ago

Then we need to consider if happi should fill in missing keys from the container with their default values. I think the answer is "probably yes"?

"Probably yes" from the above ignored the fact that the backends themselves are container-agnostic. They only deal with the data sourced from the database.

At least for now, the onus is on the user to ensure their database-stored data is in conformance with their containers. That means "don't edit the JSON files directly, or if you do please fix them up after the fact".

To that end, it was proposed that the tool happi repair should be added to make the database-stored data conform with that defined at the container level.

happi repair would do something like the following:

  1. Iterating through every database item
  2. Ensuring that the database has all keys defined in the container
  3. Prompting the user for required keys without defaults (perhaps a default could be specified on a key name-based)
  4. Saving all database items that have changed during the above process

This could/should also be an audit step. repair could have a dry run which reports issues that a happi audit could list.

tangkong commented 1 year ago

Would we say this is closed now? The happi db has ben repaired so the cxi entries are found properly.

Since search only interacts with the backend, filling container information during a search would require loading said containers, likely incurring a significant performance penalty. Is finding a performant way to do this something we'd like to pursue?

klauer commented 1 year ago

I'd say that #300 resolves it - and it's up to the user/us to ensure our manually-edited JSON files remain valid

tangkong commented 1 year ago

closed by #300