Skip to content

Fix issue setting the lookback default after it gets used #353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 15, 2025

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Apr 7, 2025

Description

PR #351 introduced a problem in kadi commands update processing where the lookback value (None by default) was being used in a computation prior to being set to the conf.default_lookback value. This was temporarily fixed in production by adding the --lookback=30 flag to the script command.

This PR does two things:

  • Move the lookback update to the top of the function before it gets used.
  • Change the test of the command archive update process to not supply the --lookback argument. This matches the way the script is called in production.

This PR also includes an unrelated fix for ruff 11.0. There are no code changes so I did not re-run unit tests.

Interface impacts

None

Testing

Unit tests

  • Mac

With only the 1st commit, the test_commands_create_archive_regress test fails with the same exception seen in production processing.

With both commits:

(ska3) ➜  kadi git:(fix-lookback-default) git rev-parse --short HEAD    
636f61e
(ska3) ➜  kadi git:(fix-lookback-default) pytest
======================================================================= test session starts ========================================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 185 items                                                                                                                                                

kadi/commands/tests/test_commands.py ..................................................................................                                      [ 44%]
kadi/commands/tests/test_filter_events.py ..                                                                                                                 [ 45%]
kadi/commands/tests/test_states.py .......................x..........................                                                                        [ 72%]
kadi/commands/tests/test_validate.py ...................                                                                                                     [ 82%]
kadi/tests/test_events.py ..........                                                                                                                         [ 88%]
kadi/tests/test_occweb.py ......................                                                                                                             [100%]

============================================================ 184 passed, 1 xfailed in 92.99s (0:01:32) =============================================================

Independent check of unit tests by Javier (also ran test after reverting 76c8551 to make sure the test would have caught the issue).

  • [OSX]:
(ska3-flight) ~/SAO/git/kadi fix-lookback-default $ git rev-parse --short HEAD    
c651a42
(ska3-flight) ~/SAO/git/kadi fix-lookback-default $ pytest kadi   
============================================================= test session starts =============================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/javierg/SAO/git/kadi
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 185 items                                                                                                                           

kadi/commands/tests/test_commands.py ..................................................................................                 [ 44%]
kadi/commands/tests/test_filter_events.py ..                                                                                            [ 45%]
kadi/commands/tests/test_states.py .......................x..........................                                                   [ 72%]
kadi/commands/tests/test_validate.py ...................                                                                                [ 82%]
kadi/tests/test_events.py ..........                                                                                                    [ 88%]
kadi/tests/test_occweb.py ......................                                                                                        [100%]

================================================== 184 passed, 1 xfailed in 79.25s (0:01:19) ==================================================

Functional tests

No functional testing.

@taldcroft taldcroft requested review from javierggt and jeanconn and removed request for jeanconn April 12, 2025 10:12
@taldcroft taldcroft merged commit a088097 into master Apr 15, 2025
4 checks passed
@taldcroft taldcroft deleted the fix-lookback-default branch April 15, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants