NSLS-II / metadatastore

DEPRECATED: Incorporated into https://github.com/NSLS-II/databroker
Other
2 stars 11 forks source link

ENH: Extend Event Document API to track external keys and filled-ness. #239

Closed danielballan closed 8 years ago

danielballan commented 8 years ago

Presently, Events don't track whether any external data has been been "filled," and it's easy to write bugs that try to fill an Event twice, blowing up on a filestore error.

The obvious solution would be to stop filling Events in place, but that would mean copying a lot of data in many common cases.

Instead, this PR introduces another parallel dict inside Event:

{'data': {'motor': 5, 'image': 'some_uid'},
 'timestamps', {'motor': ..., 'image': ...},
 'filled': {'image': False}

When Broker.fill_event operates on the document above, it will flip False to True when replacing some_uid with the image array data via filestore. Notice that fields that are not external (like 'motor' above) are not included in filled. Whether or not an Event has already been filled, the keys of the filled dict provide a quick way to determine which if any fields are external.

The new filled features has be introduced here in metadatastore, not databroker, because docts are immutable.

codecov-io commented 8 years ago

Current coverage is 86.87% (diff: 100%)

Merging #239 into master will increase coverage by 0.09%

@@             master       #239   diff @@
==========================================
  Files             7          7          
  Lines           870        876     +6   
  Methods           0          0          
  Messages          0          0          
  Branches        146        150     +4   
==========================================
+ Hits            755        761     +6   
  Misses           85         85          
  Partials         30         30          

Powered by Codecov. Last update 1a909c9...380c447

tacaswell commented 8 years ago

Does this also require a change to the document spec package?

This needs some prose docs.

attn @arkilic I think this should be transparent to the service/client, but just checking

ghost commented 8 years ago

itnshould be transparent, I'll try it out. Aside note, we better check how this affects the event retrieval performance

danielballan commented 8 years ago

This does require a change to event model if we ever validate on the way out (which we currently never do).

Where do you think these prose docs would go? MDS has none.

I don't expect this to be a significant impact on performance. Since the new dict created once per descriptor, not once per event, the cost of merely attaching it to the event should be much less than the cost of writing all the other parts of the event. But sure it doesn't hurt to test.

tacaswell commented 8 years ago

it should grow prose docs :smiling_imp:

At least dump something in https://github.com/NSLS-II/metadatastore/tree/master/doc/api-changes

cowanml commented 8 years ago

what is "filled-ness"? completeness?

danielballan commented 8 years ago

https://github.com/NSLS-II/databroker/blob/master/databroker/broker.py#L266