sot / kadi

Chandra commands and events
https://sot.github.io/kadi
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Fix bug in OBS obs_stop for cmd_evt maneuver and improve default_stop handling #319

Closed taldcroft closed 4 months ago

taldcroft commented 4 months ago

Description

OBS obs_stop

This fixes a bug where an OBS statement coming from command events (during anomaly recovery) could have the wrong obs_stop value.

When an OBS statement is created it uses the time of the next maneuver or schedule stop to set obs_stop. In normal processing those are always available, but for a maneuver in anomaly recovery neither are available and it uses a default 7 days in the future.

Normally commands which change in any way by subsequent events or loads are detecting by the diff algorithm and replace. However the obs_stop update is just part of the command params and this is not checked in the diffing. Doing so introduces false diffs due to small floating point precision issues. So this PR special-cases OBS so that the params are included in the match key.

KADI_COMMANDS_DEFAULT_STOP

An unrelated issue surfaced during testing that the KADI_COMMANDS_DEFAULT_STOP environment variable was not being applied as a filter to events the command events sheet. This was unexpected to me and would make it very difficult to do detailed "scenario testing" of stepping through the timeline of past events. In this case having the full history of command events meant that the code always had subsequent maneuvers so seeing the OBS problem was difficult.

Interface impacts

None

Testing

Unit tests

kadi/commands/tests/test_commands.py ...................................................................................... [ 37%] kadi/commands/tests/test_states.py .......................x..............................................x....................... [ 77%] kadi/commands/tests/test_validate.py .................... [ 86%] kadi/tests/test_events.py .......... [ 90%] kadi/tests/test_occweb.py ...................... [100%]

============================================================ warnings summary ============================================================= kadi/kadi/commands/tests/test_commands.py::test_get_starcats_obsid /Users/aldcroft/miniconda3/envs/ska3/lib/python3.10/site-packages/mica/utils.py:16: FutureWarning: ska_load_dir() is deprecated. Use parse_cm.paths.load_dir_from_load_name() dir_parts = ska_load_dir(load_name).parts

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ========================================== 230 passed, 2 xfailed, 1 warning in 99.43s (0:01:39) ===========================================

Independent check of unit tests by Jean
- [x] OSX

(ska3) flame:kadi jean$ pytest ===================================================================== test session starts ===================================================================== platform darwin -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0 rootdir: /Users/jean/git, configfile: pytest.ini plugins: timeout-2.1.0, anyio-3.6.2 collected 232 items

kadi/commands/tests/test_commands.py .......s.......s.....................................................................s [ 37%] kadi/commands/tests/test_states.py .......................x..............................................x....................... [ 77%] kadi/commands/tests/test_validate.py .................... [ 86%] kadi/tests/test_events.py .......... [ 90%] kadi/tests/test_occweb.py ...................... [100%]

====================================================================== warnings summary ======================================================================= kadi/kadi/commands/tests/test_commands.py::test_get_starcats_each_year[year0] /Users/jean/miniconda3/envs/ska3/lib/python3.10/site-packages/setuptools_scm/git.py:295: UserWarning: git archive did not support describe output warnings.warn("git archive did not support describe output")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html =============================================== 227 passed, 3 skipped, 2 xfailed, 1 warning in 81.41s (0:01:21) =============================================== (ska3) flame:kadi jean$ git rev-parse HEAD 3301432d93e8dbd79418873e076abab68a1a9170


### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
Detailed functional testing in `validate/pr319`. This demonstrates that the patch fixes the problem. This functional validation should work on HEAD.

I did this before committing the script. To replicate copy the script to a different name.

git switch master
python run_updates.py > master.log git switch fix-matching-for-obs-cmds python run_updates.py > fix-matching-for-obs-cmds.log

jeanconn commented 4 months ago

The only thing that niggles at me is that KADI_COMMANDS_DEFAULT_STOP is not really a default (if it has a value, we're doing something special that is non-default) but that is neither here nor there.