Inconsistent results / exceptions in starcheck queries
Closed this issue · 8 comments
This original issue was user error, but subsequent discussion shows inconsistencies in the way that queries for not-available starcheck products are handled. Would be good to clean up, and in particular raise an exception any time a valid query result cannot be returned.
In [28]: get_starcat(21082)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
/data/baffin/tom/git/proseco/proseco/tests/starcheck_proseco.py in <module>()
----> 1 get_starcat(21082)
/proj/sot/ska3/flight/arch/x86_64-linux_CentOS-5/lib/python3.6/site-packages/mica/starcheck/starcheck.py in get_starcat(obsid)
34 """
35 if obsid not in OBS_CACHE:
---> 36 OBS_CACHE[obsid] = get_starcheck_catalog(obsid)
37 return OBS_CACHE[obsid]['cat']
38
/proj/sot/ska3/flight/arch/x86_64-linux_CentOS-5/lib/python3.6/site-packages/mica/starcheck/starcheck.py in get_starcheck_catalog(obsid, mp_dir, starcheck_db, timelines_db)
346 status = None
347 if mp_dir is None:
--> 348 mp_dir, status, obs_date = get_mp_dir(obsid, starcheck_db=starcheck_db, timelines_db=timelines_db)
349 # if it is still none, there's nothing to try here
350 if mp_dir is None:
/proj/sot/ska3/flight/arch/x86_64-linux_CentOS-5/lib/python3.6/site-packages/mica/starcheck/starcheck.py in get_mp_dir(obsid, starcheck_db, timelines_db)
308 else:
309 return (sc['dir'], 'approved', sc_date)
--> 310 raise ValueError("get_mp_dir should find something or nothing")
311
312
ValueError: get_mp_dir should find something or nothing
21082 was an obsid that was scheduled in APR2318 A and B, but pulled from the schedule as-run for APR2318C. It still hasn't been observed. As such, it doesn't have an "as-run" configuration and there's no default last fetch for it from get_starcheck_catalog. So, I could improve the error message out of get_starcheck_catalog, but if you ask for an obsid that does exist in the database but didn't actually run (and it is now after the time it should have run according to the potential A and B catalogs). you'll get something like this.
If you are running regress tests using APR2318A or APR2318B, you should probably use get_starcheck_catalog(21082, mp_dir='/2018/APR2318/oflsb/")
I suppose I don't know if this works well with the OBS_CACHE.
There is an inconsistency I am tempted to fix, i.e. make the first case raise an exception.
In [13]: get_starcheck_catalog(21082, '/2018/APR2318/oflsc/')
Out[13]:
{'mp_dir': '/2018/APR2318/oflsc/',
'status': None,
'obs': None,
'manvr': [],
'warnings': [],
'cat': []}
In [14]: get_starcheck_catalog(21082)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-14-10d28336f145> in <module>()
----> 1 get_starcheck_catalog(21082)
~/git/mica/mica/starcheck/starcheck.py in get_starcheck_catalog(obsid, mp_dir, starcheck_db, timelines_db)
349 status = None
350 if mp_dir is None:
--> 351 mp_dir, status, obs_date = get_mp_dir(obsid, starcheck_db=starcheck_db, timelines_db=timelines_db)
352
353 # if it is still none, there's nothing to try here
~/git/mica/mica/starcheck/starcheck.py in get_mp_dir(obsid, starcheck_db, timelines_db)
305 else:
306 return (sc['dir'], 'approved', sc_date)
--> 307 raise ValueError("get_mp_dir should find something or nothing")
308
309
ValueError: get_mp_dir should find something or nothing
Or likewise this should really be an exception:
In [26]: starcheck.get_starcat(2108211, 'APR2318C')
Out[26]: []
But I think I am going to stop with #178.
I think throwing an exception there would not break anything else, but I do not recall if there is code that handles "nothing was returned in the query for that obsid in this week" that would need to handle the new exception you'd like to raise.
All of this is old spaghetti code, but I think we want to do the starcheck improvements -> starcheck real useful JSON -> new database before we improve all of this.
And this:
In [4]: print(starcheck.get_starcheck_catalog(21082, '/junk/'))
None
OK, I just edited the issue description and renamed it.
I think that cleaning up the handling of bad queries could be a short afternoon project. I'm actually using this module (a lot!) and being error prone I am getting bit by the issues raised here.