Open
Conversation
2 tasks
jeanconn
reviewed
Jan 22, 2026
jzuhone
approved these changes
Jan 23, 2026
Collaborator
jzuhone
left a comment
There was a problem hiding this comment.
Looks good except the minor nit about the commented-out lines, and this behavior is OK for ACIS Ops.
| pytest.skip("No backstop file found") | ||
|
|
||
| # read_backstop(add_observations=True) requires internet because it calls | ||
| # kcs.get_continuity(). |
Collaborator
There was a problem hiding this comment.
Will you remove the commented-out lines or will they be needed?
Member
Author
There was a problem hiding this comment.
This is a code comment justifying the pytest skipif above, not a commented-out line.
3b929d6 to
e348949
Compare
Member
Author
|
@jeanconn - this is ready for review again, with an additional change to remove the Note that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is a potentially impacting PR to be strict about the scenario if there is no internet. The driver here is to avoid silently providing incorrect results for commands or states for no internet.
The current behavior is that the default
scenario=Nonewill fall back to using the commands available in the archive ($SKA/data/kadi/cmds2.h5) if there is no internet. That was put in place intentionally so things would not break horribly without internet, but in hindsight avoids the golden rule to "not guess". This falling-back behavior applies to the newcustomcommand sheet as well, which is simply ignored if there is no internet. This caused the regression test failure on cheru.This PR changes
get_cmdsto requirescenario="flight"if there is no internet. Other commands likeget_observations,get_states,get_continuityall callget_cmds, so the new requirement applies across the board to all kadi commands operationsIt also removes the
kadi.commands.commands_v2.HAS_INTERNETglobal constant. This was previously set at import time, but since internet can come and go, that strategy was inherently unreliable.Interface impacts
Requires
scenario="flight"if there is no internet. This can be set globally via the environment variableKADI_SCENARIO="flight"(either programatically or in a shell).This might require changes in downstream code, e.g.
mica.starcheck.get_starcheck_catalogends up callingget_observations, so now that would fail without internet.Testing
Unit tests
With Internet
With no internet
Independent check of unit tests by [REVIEWER NAME]
Functional tests
No functional testing.