fix: rework ops.main type hints to allow different flavours (callable module)

[dimaqq]

Rework the callable module to support any of the existing ops entry point invocations:

# good type hints, works at run time
import ops; ops.main(C)
import ops; ops.main.main(C)
import ops.main; main(C)
import ops.main; main.main(C)
from ops import main; main(C)
from ops import main; main.main(C)
from ops.main import main; main(C)

# impossible to type hint, ok at run time
import ops.main; ops.main(C)

Manual mypy tests:

• nginx-ingress-integrator-operator mypy 1.6.1 OK
• sdcore-webui-operator mypy 1.11.1 OK

Super-tox: 114/115 (ran on host)

One lint failure in hardware-observer-operator:

• src/charm.py:290: error: Unused "type: ignore" comment [unused-ignore]

Code in question:

import ops

...

if __name__ == "__main__":  # pragma: nocover
    ops.main(HardwareObserverCharm)  # type: ignore

This is mypy complaining, because this project includes warn_unused_ignores = true in [tool.mypy] settings.

[dimaqq]

Some tests fail under py 3.8, investigating...

[tonyandrewmeyer]

Rework the callable module to support any of the existing ops entry point invocations:

import ops; ops.main(C)
import ops; ops.main.main(C)
import ops.main; main(C)
import ops.main; main.main(C)
from ops import main; main(C)
from ops import main; main.main(C)
from ops.main import main; main(C)

We don't actually want people doing these other than two of them, though. Is the purpose of the PR to allow more ways to invoke main? I don't see why we would want that. Or is this to improve the type annotation situation? If that's the case it seems good, but we should have the PR description say that.

Minor note: this PR is built on top of #1327

What's the story now given that #1327 is closed not merged?

[tonyandrewmeyer]

Nice! It will be great to get rid of those # type:ignores in all the charms.

A couple of small queries/suggestions.

[dimaqq]

Re failing tests: this is a bit weird... I need to check if maybe something got unmocked wrong or some other inter-test dependency is at play. I've tried a few of these tests and they pass individually.

[dimaqq]

I was able to reproduce the GHA test failure. Example:

uvx -p 3.10 tox -e unit -- -k old_key

Fails under Python 3.8, 3.9, 3.10;
Succeeds under Python 3.11, 3.12, 3.13.

Edit: fixed now. I understand the failure and I'm actually surprised how on earth it worked under newer Python. Perhaps unittest.mock.patch got really really clever in py 3.11.

[dimaqq]

Doc fail:

git clone --depth 1 https://github.com/canonical/operator .
...
git checkout --force aca644874cd9ff28fd85bd9e9cd3238ca89cb0d5
fatal: reference is not a tree: aca644874cd9ff28fd85bd9e9cd3238ca89cb0d5

Now wonder when this commit is in my fork, not canonical.

[dimaqq]

What's the story now given that #1327 is closed not merged?

That PR became irrelevant after JujuContext landed.

[dimaqq]

We don't actually want people doing these other than ...

True, but we don't want to break existing charms, do we?
Note that today, ops.main.main() type is fine, and some charms implicitly rely on that.

[tonyandrewmeyer]

True, but we don't want to break existing charms, do we? Note that today, ops.main.main() type is fine, and some charms implicitly rely on that.

Yes, I agree completely that we have to support any way of calling main that we currently do now. But we don't need to remove the need to use type: ignore on anything other than the two ways we actually want people to do it. If we do, then that's fine as long as it doesn't make the solution more complex than it otherwise needs to be.

[tonyandrewmeyer]

Looks good, thanks - just a couple of small queries.

Thanks for doing the broad test against charms (super_tox) - it's good to be confident in how this will impact everyone.

[tonyandrewmeyer]

Is there any need to have this and not just use CharmBase?

[tonyandrewmeyer]

It seems like maybe there were meant to be more words there?

[dimaqq]

Closing in favour of #1345