Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: daf_persistence
-
Labels:None
-
Story Points:2
-
Epic Link:
-
Sprint:DB_F18_10
-
Team:Data Access and Database
Description
If you supply insufficient data in the dataId for a butler to put() the data, it will just pretend to have done it (i.e. it doesn't raise or warn, it just silently returns as if all was fine).
If one does
butler.put(img, 'postISRCCD', dataId={'visit':123,'filter':'r'})
|
where the postISRCCD template looks like
template: "postISRCCD/postISRCCD_v%(visit)d_f%(filter)s.fits"
then this will work. However, if you provide an empty dataId dict, it will not complain, but nothing will get written (of course).
This seems like a moderately serious problem, as it is easy to accidentally under-specify or mis-specify a dataId.
(FWIW, if I change 'postISRCCD' to an undefined dataset type I do get an error).
Attachments
Activity
Paul Price
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Priority | Undefined [ 10000 ] | Critical [ 2 ] |
| Team | Data Access and Database [ 10204 ] |
Sounds like we should ask Fritz Mueller to comment. Clearly, this is a Bad Thing, but implementation priority probably depends on how many free cycles his team has.
We do have some effort budget for supporting BG2 issues that are affecting current usage. If we want to spend some of that on this issue we'll need:
- Concensus on what the new/fixed behavior should be (without significantly impacting other current BG2 usage)
- An effort estimate on the fix from Nate Pease [X]
Well, my vote would be for the minimal, quick-fix, i.e. just Raise, so that it doesn't fail silently.
+100
KT points out a probably-easy fix:
Either check for an empty locations list in https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/butler.py#L1375 or https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/butler.py#L1451
If that works, to bring up environment, repro, code, write tests, & merge is probably on the order of 1 to 2 points (I updated points field)
| Story Points | 2 |
Okay, great – interest seems high enough and the cost low enough that we'll schedule it.
Let's see how some of the other BG2-related threads from today tail out and maybe we can batch a couple of these to reduce context-switch overhead for Nate.
I HATE THIS BUG!!!
(Just shouting on Jira tickets is a legit way of raising concerns, right?)
Hey Fritz Mueller — as of March it sounded like we might actually get this fixed. Any progress?
I mean, I really don't want people to waste time on something if it's only going to be replaced by Gen3 and soon, and if this will take any significant time, and I've worked around it for now, of course, but if it's quite easy to make it raise or at least say something, that would be quite nice 
Also, as was being said in the butler-room, given the existence of butler.getUri(), it's now trivial to test whether the .put() will be successful, so, although a little ugly, that could just be put as a test-and-raise/warn in the meantime with presumably minimal effort...
| Assignee | Nate Pease [ npease ] |
I'll work on this now...
| Sprint | DB_F18_10 [ 792 ] |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Reviewers | Eli Rykoff [ erykoff ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
| Epic Link |
|
Just got bitten by this as well. John Swinbank should we push this up the stack?