operator icon indicating copy to clipboard operation
operator copied to clipboard

Passing check level from layer to CheckInfo causes type error

Open dwilding opened this issue 9 months ago • 1 comments

The testing code:

ctx = testing.Context(MyCharm, meta={"name": "foo", "containers": {"my_container": {}}})
layer = pebble.Layer({
    "checks": {"http-check": {"override": "replace", "startup": "enabled", "failures": 7}},
})
check_info = testing.CheckInfo(
    "http-check",
    status=pebble.CheckStatus.UP,
    level=layer.checks["http-check"].level,
    startup=layer.checks["http-check"].startup,
    threshold=layer.checks["http-check"].threshold,
)
container = testing.Container("my_container", check_infos={check_info}, layers={"layer1": layer})
state = testing.State(containers={container})
ctx.run(ctx.on.pebble_check_failed(info=check_info, container=container), state=state)

produces an error from the type checker:

error: Argument of type "CheckLevel | str" cannot be assigned to parameter "level" of type "CheckLevel | None" in function "__init__"

It's possible to work around the issue by changing level=layer.checks["http-check"].level to level=pebble.CheckLevel(layer.checks["http-check"].level) (or directly hard-coding level=pebble.CheckLevel.READY).

The proposal to fix this in ops.testing is:

change testing.CheckInfo so that level (maybe some of the others?) accepts str as well but probably converts to a CheckLevel in __post_init__

See https://github.com/canonical/charmcraft/pull/2309#issuecomment-2942189138

dwilding avatar Jun 05 '25 02:06 dwilding

Other suggestions from the discussion in daily:

  • Provide defaults in testing.CheckInfo matching the Pebble ones, and then be explicit duplicating the value if using something else than the default.
  • Change the docs so that we don't recommend getting the value from the layer like Pebble does but instead just always hard-code/duplicate the values.
  • Add some sort of testing.CheckInfo.from_layer(failures) method that would give back an appropriate object with all the values copied from the layer or computed (status) or passed in (failures).
  • Change the docs to use the workaround from the description.

tonyandrewmeyer avatar Jun 05 '25 02:06 tonyandrewmeyer