sot / kadi

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

Remove commands v1 from kadi #328

Closed taldcroft closed 2 months ago

taldcroft commented 3 months ago

Description

This PR entirely removes commands v1 from kadi.

This includes some refactoring now that there is only one version of kadi commands.

The test coverage now in the no-internet case is improved with this PR. When there is no internet, kadi commands v2 get_cmds behaves the same as scenario="flight". In test_commands.py, previously many of the v2 tests were omitted for no internet. Now they are run using the flight archive only.

This PR supersedes #327.

Interface impacts

Commands v1 has been deprecated for a long time and now it is fully removed. The configuration setting commands_version is removed and the environment variable KADI_COMMANDS_VERSION no longer has any effect.

Testing

Unit tests

kadi/commands/tests/test_commands.py .................................................................. [ 37%] ............. [ 44%] kadi/commands/tests/test_states.py .......................x....................... [ 70%] kadi/commands/tests/test_validate.py .................... [ 82%] kadi/tests/test_events.py .......... [ 87%] kadi/tests/test_occweb.py ...................... [100%]

============================ 177 passed, 1 xfailed, 2 warnings in 74.43s (0:01:14) ============================

(ska3) ➜ kadi git:(remove-cmds-v1) git rev-parse HEAD
355b284f7500790e90d755a09496a54db2db2c79


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

ska3-jeanconn-fido> pytest ====================================================== test session starts ====================================================== platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0 rootdir: /proj/sot/ska/jeanproj/git configfile: pytest.ini plugins: anyio-4.3.0, timeout-2.2.0 collected 178 items

kadi/commands/tests/test_commands.py ............................................................................... [ 44%] kadi/commands/tests/test_states.py .......................x....................... [ 70%] kadi/commands/tests/test_validate.py .................... [ 82%] kadi/tests/test_events.py .......... [ 87%] kadi/tests/test_occweb.py ...................... [100%]

======================================================= warnings summary ======================================================== kadi/kadi/commands/tests/test_commands.py::test_get_starcats_each_year[year0] /proj/sot/ska3/flight/lib/python3.11/site-packages/setuptools_scm/git.py:308: 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 ===================================== 177 passed, 1 xfailed, 1 warning in 173.05s (0:02:53) ===================================== ska3-jeanconn-fido> git rev-parse HEAD 355b284f7500790e90d755a09496a54db2db2c79 ska3-jeanconn-fido>


### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
Also ran and passed all tests with no internet by turning off WiFi.

(ska3) ➜ kadi git:(remove-cmds-v1) pytest
============================================= test session starts ============================================= platform darwin -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0 rootdir: /Users/aldcroft/git configfile: pytest.ini plugins: timeout-2.2.0, anyio-4.3.0 collected 178 items

kadi/commands/tests/test_commands.py .......sssssss..ss...............................................s [ 37%] ss..s........ [ 44%] kadi/commands/tests/test_states.py .......................x....................... [ 70%] kadi/commands/tests/test_validate.py .sssssssssssssssssss [ 82%] kadi/tests/test_events.py .......... [ 87%] kadi/tests/test_occweb.py ssssssssssssssssssssss [100%]

============================================== warnings summary ===============================================

jeanconn commented 3 months ago

As I noted in https://github.com/sot/kadi/pull/327#issuecomment-2032140785 I'm not sure if there is a use case to continue to skip the v2 tests if the machine doesn't have internet (sometimes a greta machine).

jeanconn commented 3 months ago

I think there are still a few stray docs-things that were already outdated like

https://github.com/sot/kadi/blob/355b284f7500790e90d755a09496a54db2db2c79/kadi/commands/core.py#L188

And the reference still in

https://github.com/sot/kadi/blob/remove-cmds-v1/docs/api_commands.rst

I'm guessing from the way you've completely removed the v1 docs that removing them completely is the strategy you want, which seems fine.