Revert the change to decorate `Validate.update_tlm` method

taldcroft commented 7 months ago

Description

This is a critical fix that reverts a change made in #268. Unfortunately I did not appreciate what this actually meant and our testing does not cover this code at all. Without the fix the kadi_validate_states script fails to run at all.

I plan to add a unit test that does a regression test of the validate code, but in the meantime this PR uses functional testing.

Interface impacts

None

Testing

Unit tests

[x] Mac


(ska3-perf) ➜  kadi git:(revert-abstract-method) git rev-parse HEAD                  
b2e4f579b4ac6bee3be176c4d10272e07b310d4f
(ska3-perf) ➜  kadi git:(revert-abstract-method) pytest kadi
==================================================== test session starts ====================================================
platform darwin -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/aldcroft/git, configfile: pytest.ini
plugins: timeout-2.1.0, anyio-3.6.2
collected 199 items

kadi/commands/tests/test_commands.py ........................................................................... [ 37%] kadi/commands/tests/test_states.py ......................x.............................................x............. [ 78%] .......... [ 83%] kadi/tests/test_events.py .......... [ 88%] kadi/tests/test_occweb.py ...................... [100%]

======================================== 197 passed, 2 xfailed in 101.15s (0:01:41) =========================================


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

### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
Compared the output of this branch with ska3-performance (effectively masters) to the output of the current flight release 7.6.0 in ska3. The results (see below) show no substantive differences. 

Branch run in ska3-performance:

python -m kadi.scripts.validate_states --out-dir test-revert --stop=2023:311 --days 3.5 >& test-revert.log

Output: https://icxc.cfa.harvard.edu/aspect/test_review_outputs/kadi/pr299/test-revert/

Flight run in ska3

kadi_validate_states --out-dir test-flight --stop=2023:311 --days 3.5 >& test-flight.log

Output: https://icxc.cfa.harvard.edu/aspect/test_review_outputs/kadi/pr299/test-flight/

#### Diff of run logs after removing log timestamps

(ska3-perf) ➜ kadi git:(revert-abstract-method) diff test-flight.log test-revert.log 86c86 < **** get_cmds: Getting commands from 815400069.5749999 to 815702469.175 for scenario=None

get_cmds: Getting commands from 2023:307:12:00:00.391 to 2023:310:23:59:59.991 for scenario=None 129c129 < get_cmds: Getting commands from <chandra_time.Time.DateTime object at 0x1332bb220> to <chandra_time.Time.DateTime object at 0x1332ba590> for scenario=None

**** get_cmds: Getting commands from 2023:300:12:00:00.391 to 2023:307:12:00:00.391 for scenario=None ...

A bunch more like that

jeanconn commented 7 months ago

I note that the failure this references looks like

(ska3-masters) jeanconn-fido> kadi_validate_states --out-dir test-masters --stop=2023:311 --days 3.5
2023-11-21 22:41:45,692 get_index_page_html: Validating pitch
Traceback (most recent call last):
  File "/fido.real/conda/envs/ska3-masters/bin/kadi_validate_states", line 33, in <module>
    sys.exit(load_entry_point('kadi==7.7.0', 'console_scripts', 'kadi_validate_states')())
  File "/fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/kadi/scripts/validate_states.py", line 74, in main
    html = validate.get_index_page_html(opt.stop, opt.days, opt.states, opt.no_exclude)
  File "/fido.real/conda/envs/ska3-masters/lib/python3.10/site-packages/kadi/commands/validate.py", line 795, in get_index_page_html
    validator: Validate = cls(stop=stop, days=days, no_exclude=no_exclude)
TypeError: Can't instantiate abstract class ValidatePitch with abstract method update_tlm

taldcroft commented 7 months ago

@jeanconn - I've fixed the description to show the correct command for functional testing.

jeanconn commented 7 months ago

kadi.commands.validate does exist but I'm confused because I don't think it has a main so it doesn't appear to do anything with the updated cmdline either.

taldcroft commented 7 months ago

@jeanconn - sorry, brain misfiring there.

(ska3-perf) ➜  kadi git:(validate-unit-test) ✗ python -m kadi.scripts.validate_states --help
usage: validate_states.py [-h] [--stop STOP] [--days DAYS] [--log-level LOG_LEVEL] [--out-dir OUT_DIR] [--state STATES]
                          [--no-exclude] [--in-work] [--version]

Validate kadi command states

options:
  -h, --help            show this help message and exit
  --stop STOP           Stop date for update (default=Now)
  --days DAYS           Lookback days (default=14 days)
  --log-level LOG_LEVEL
                        Logging level (DEBUG | INFO | WARNING, default=INFO)
  --out-dir OUT_DIR     Output directory for index.html (default='.')
  --state STATES        State(s) to validate (default=ALL)
  --no-exclude          Do not apply exclude intervals from validation (for testing)
  --in-work             Include in-work events in validation (for checking new events)
  --version             show program's version number and exit

sot / kadi