obs_base/DM-8230: Strip skymap data ID keys from registry queries. #13
Conversation
| fastPath = True | ||
| for p in properties: | ||
| if p not in ('filter', 'expTime', 'taiObs'): | ||
| fastPath = False | ||
| break | ||
| if fastPath and 'visit' in dataId and "raw" in self.tables: | ||
| lookupDataId = {'visit': dataId['visit']} | ||
| self.registry.lookup(properties, 'raw_visit', lookupDataId) |
There was a problem hiding this comment.
was this effectively a no-op before?
There was a problem hiding this comment.
I hadn't noticed that fully until now, but I think it was indeed an unintentional no-op - I think it was supposed to return on this line, which would indeed have short-circuited the rest.
I also think this code path is very rarely invoked, because the all butler methods that eventually lead to lookup calls tend to include all data ID keys (including e.g. visit and ccd) in properties, not just the missing ones.
There was a problem hiding this comment.
It looks like it might have gone from optimization to no-op in e522602. The intent was for it to be invoked fairly frequently, but perhaps that also went by the wayside a while ago.
TallJimbo
force-pushed
the
tickets/DM-8230
branch
from
d1a3a5e
to
42d5f8d
Compare
This change could be considerably simplified by changing the API of mapping.lookup() to return via a dict instead of a list, but for now I feel it's better to leave this as a self-contained change that doesn't break any other code.
TallJimbo
force-pushed
the
tickets/DM-8230
branch
from
42d5f8d
to
615be1a
Compare
This change could be considerably simplified by changing the API of mapping.lookup() to return via a dict instead of a list, but for now I feel it's better to leave this as a self-contained change that
doesn't break any other code.