From 21b7f48b39839f83c2ddb8348d922149de0da540 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 23 Aug 2024 17:28:58 +0900 Subject: [PATCH 01/33] newer style annotations --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 58a7789e0..4bfa2d1db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,6 +141,12 @@ ignore = [ # Manual dict comprehension. "PERF403", + # Convert Dict[X] to dict[x], which we cannot do while we support Python 3.8 + "UP006", + + # Convert Union[X, Y] to X | Y, which we cannot do while we support Python 3.8 + "UP007", + # Convert {} from `TypedDict` functional to class syntax # Note that since we have some `TypedDict`s that cannot use the class # syntax, we're currently choosing to be consistent in syntax even though From 96b4edcf9dd21fd6f33be3a8c6b6cadeb03b1b67 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 27 Aug 2024 16:16:22 +0900 Subject: [PATCH 02/33] use keep-runtime-typing --- pyproject.toml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4bfa2d1db..58a7789e0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,12 +141,6 @@ ignore = [ # Manual dict comprehension. "PERF403", - # Convert Dict[X] to dict[x], which we cannot do while we support Python 3.8 - "UP006", - - # Convert Union[X, Y] to X | Y, which we cannot do while we support Python 3.8 - "UP007", - # Convert {} from `TypedDict` functional to class syntax # Note that since we have some `TypedDict`s that cannot use the class # syntax, we're currently choosing to be consistent in syntax even though From 8ce4210d0100d4b19d24f920b2197bee9a62988b Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 23 Aug 2024 17:28:58 +0900 Subject: [PATCH 03/33] newer style annotations --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 58a7789e0..4bfa2d1db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,6 +141,12 @@ ignore = [ # Manual dict comprehension. "PERF403", + # Convert Dict[X] to dict[x], which we cannot do while we support Python 3.8 + "UP006", + + # Convert Union[X, Y] to X | Y, which we cannot do while we support Python 3.8 + "UP007", + # Convert {} from `TypedDict` functional to class syntax # Note that since we have some `TypedDict`s that cannot use the class # syntax, we're currently choosing to be consistent in syntax even though From 6148420f8a6466848660f07564b97a74c23b0f12 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 27 Aug 2024 16:16:22 +0900 Subject: [PATCH 04/33] use keep-runtime-typing --- pyproject.toml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4bfa2d1db..58a7789e0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,12 +141,6 @@ ignore = [ # Manual dict comprehension. "PERF403", - # Convert Dict[X] to dict[x], which we cannot do while we support Python 3.8 - "UP006", - - # Convert Union[X, Y] to X | Y, which we cannot do while we support Python 3.8 - "UP007", - # Convert {} from `TypedDict` functional to class syntax # Note that since we have some `TypedDict`s that cannot use the class # syntax, we're currently choosing to be consistent in syntax even though From 7e8493c2913ba6ff975bdf38850d28e446e1fbdc Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 09:58:05 +0900 Subject: [PATCH 05/33] fix: rework ops.main type hints to allow different import flavours --- ops/__init__.py | 7 +- ops/main.py | 17 ++-- test/test_main.py | 4 +- test/test_main_invocations.py | 152 ++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 16 deletions(-) create mode 100644 test/test_main_invocations.py diff --git a/ops/__init__.py b/ops/__init__.py index 8fccdc619..2d160cb32 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -182,9 +182,7 @@ # import was here previously from . import charm # type: ignore # noqa: F401 `.charm` imported but unused -# Import the main module, which we've overriden in main.py to be callable. -# This allows "import ops; ops.main(Charm)" to work as expected. -from . import main +from . import main as _main # Explicitly import names from submodules so users can just "import ops" and # then use them as "ops.X". @@ -321,3 +319,6 @@ # rather than a runtime concern. from .version import version as __version__ + +# This allows "import ops; ops.main(Charm)" to work as expected. +main = _main._CallableMainModule('ops.main', _main.__doc__) diff --git a/ops/main.py b/ops/main.py index d232a7fd2..a255fc66b 100644 --- a/ops/main.py +++ b/ops/main.py @@ -26,6 +26,7 @@ import sys import warnings from pathlib import Path +from types import ModuleType from typing import Any, Dict, List, Optional, Tuple, Type, Union, cast import ops.charm @@ -555,14 +556,8 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[ sys.exit(e.exit_code) -# Make this module callable and call main(), so that "import ops" and then -# "ops.main(Charm)" works as expected now that everything is imported in -# ops/__init__.py. Idea from https://stackoverflow.com/a/48100440/68707 -class _CallableModule(sys.modules[__name__].__class__): - def __call__( - self, charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None - ): - return main(charm_class, use_juju_for_storage=use_juju_for_storage) - - -sys.modules[__name__].__class__ = _CallableModule +# Support old and new style main calls at run time and for type checking +# - ops.main.main(SomeCharm) +# - ops.main(SomeCharm) +class _CallableMainModule(ModuleType): # pyright: ignore[reportUnusedClass] as it's used in __init__.py + __call__ = main = staticmethod(main) diff --git a/test/test_main.py b/test/test_main.py index 98ed5fb1b..fddcd1d24 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -148,7 +148,7 @@ def _check( with fake_metadata.open('wb') as fh: fh.write(b'name: test') - ops.main(charm_class, **kwargs) # type: ignore + ops.main(charm_class, **kwargs) def test_init_signature_passthrough(self): class MyCharm(ops.CharmBase): @@ -236,7 +236,7 @@ def __init__(self, framework: ops.Framework): with patch.dict(os.environ, fake_environ): with patch('ops.main._emit_charm_event') as mock_charm_event: - ops.main(MyCharm) # type: ignore + ops.main(MyCharm) assert mock_charm_event.call_count == 1 return mock_charm_event.call_args[0][1] diff --git a/test/test_main_invocations.py b/test/test_main_invocations.py new file mode 100644 index 000000000..73805402b --- /dev/null +++ b/test/test_main_invocations.py @@ -0,0 +1,152 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Verify that `ops.main` can be invoked in every possible way. + +Validate that main can be called at runtime and according to the static type checker. + +Runtime tests: +- Ensure that `ops.main` and `ops.main.main` can be invoked with a charm class. +- Across 3 import styles: ops, main from ops, main from ops.main. +- Confirm that calling main without a charm class fails. + +Typing tests: +- Ensure that `ops.main` and `ops.main.main` are callable with correct argument. +- Across same 3 import styles as above. +- Confirm that calling main without a charm class is caught by static analysis. +""" + +import os +from pathlib import Path +from typing import Callable, Type +from unittest.mock import Mock + +import pytest + +import ops + +Reset = Callable[[], None] + + +def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): + """Usage: + from somewhere import main + type_test_dummy(main) + """ + + +def type_test_negative(_arg: Callable[[], None]): + """Usage: + from somewhere import main + type_test_negative(main) # type: ignore + + The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, + should the passed argument match the expected argument type. + """ + + +@pytest.fixture +def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + monkeypatch.setattr('sys.argv', ('hooks/install',)) + monkeypatch.setattr('ops._main._emit_charm_event', Mock()) + monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock()) + monkeypatch.setattr('ops.charm._evaluate_status', Mock()) + monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path)) + monkeypatch.setenv('JUJU_UNIT_NAME', 'test_main/0') + monkeypatch.setenv('JUJU_MODEL_NAME', 'mymodel') + monkeypatch.setenv('JUJU_DISPATCH_PATH', 'hooks/install') + monkeypatch.setenv('JUJU_VERSION', '3.5.0') + (tmp_path / 'metadata.yaml').write_text('name: test', encoding='utf-8') + (tmp_path / 'dispatch').absolute().touch(mode=0o755) + + yield (reset := lambda: os.environ.pop('OPERATOR_DISPATCH', None)) + + reset() + + +class IdleCharm(ops.CharmBase): + def __init__(self, framework: ops.Framework): + super().__init__(framework) + + +def test_top_level_import(reset: Reset): + import ops + + type_test_dummy(ops.main.__call__) # pyright is quirky + type_test_dummy(ops.main.main) + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + ops.main(IdleCharm) + + reset() + ops.main.main(IdleCharm) + + with pytest.raises(TypeError): + ops.main() # type: ignore + + with pytest.raises(TypeError): + ops.main.main() # type: ignore + + +def test_submodule_import(reset: Reset): + import ops.main + + type_test_dummy(ops.main.__call__) # type: ignore FIXME + type_test_dummy(ops.main.main) + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + ops.main(IdleCharm) # type: ignore FIXME + + reset() + ops.main.main(IdleCharm) + + with pytest.raises(TypeError): + ops.main() # type: ignore + + with pytest.raises(TypeError): + ops.main.main() # type: ignore + + +def test_import_from_top_level_module(reset: Reset): + from ops import main + + type_test_dummy(main.__call__) + type_test_dummy(main.main) + type_test_negative(main.__call__) # type: ignore + type_test_negative(main.main) # type: ignore + + main(IdleCharm) + + reset() + main.main(IdleCharm) + + with pytest.raises(TypeError): + main() # type: ignore + + with pytest.raises(TypeError): + main.main() # type: ignore + + +def test_import_from_submodule(reset: Reset): + from ops.main import main + + type_test_dummy(main) + type_test_negative(main) # type: ignore + + main(IdleCharm) + + with pytest.raises(TypeError): + main() # type: ignore From 7d351b640ffe2e521bbc8548ef84c2966d3bd929 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 27 Aug 2024 17:36:40 +0900 Subject: [PATCH 06/33] mock things correctly --- test/test_main.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_main.py b/test/test_main.py index fddcd1d24..3a0671619 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -96,8 +96,8 @@ def __init__( self.check_name = check_name -@patch('ops.main.setup_root_logging', new=lambda *a, **kw: None) # type: ignore -@patch('ops.main._emit_charm_event', new=lambda *a, **kw: None) # type: ignore +@patch('ops._main.setup_root_logging', new=lambda *a, **kw: None) # type: ignore +@patch('ops._main._emit_charm_event', new=lambda *a, **kw: None) # type: ignore @patch('ops.charm._evaluate_status', new=lambda *a, **kw: None) # type: ignore class TestCharmInit: @patch('sys.stderr', new_callable=io.StringIO) @@ -203,7 +203,7 @@ def test_controller_storage_deprecated(self): @patch('sys.argv', new=('hooks/config-changed',)) -@patch('ops.main._Manager._setup_root_logging', new=lambda *a, **kw: None) # type: ignore +@patch('ops._main._Manager._setup_root_logging', new=lambda *a, **kw: None) # type: ignore @patch('ops.charm._evaluate_status', new=lambda *a, **kw: None) # type: ignore class TestDispatch: def _check(self, *, with_dispatch: bool = False, dispatch_path: str = ''): @@ -235,7 +235,7 @@ def __init__(self, framework: ops.Framework): dispatch.chmod(0o755) with patch.dict(os.environ, fake_environ): - with patch('ops.main._emit_charm_event') as mock_charm_event: + with patch('ops._main._emit_charm_event') as mock_charm_event: ops.main(MyCharm) assert mock_charm_event.call_count == 1 From b3e14da4cadba1847903d14da415e94d77eb026a Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 23 Aug 2024 17:28:58 +0900 Subject: [PATCH 07/33] newer style annotations --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 58a7789e0..4bfa2d1db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,6 +141,12 @@ ignore = [ # Manual dict comprehension. "PERF403", + # Convert Dict[X] to dict[x], which we cannot do while we support Python 3.8 + "UP006", + + # Convert Union[X, Y] to X | Y, which we cannot do while we support Python 3.8 + "UP007", + # Convert {} from `TypedDict` functional to class syntax # Note that since we have some `TypedDict`s that cannot use the class # syntax, we're currently choosing to be consistent in syntax even though From 8a786b986da200b979a4c11f97078031a16aedcb Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 22 Aug 2024 15:58:59 +0900 Subject: [PATCH 08/33] fix: rework ops.main type hints to allow different import flavours --- test/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_main.py b/test/test_main.py index 3a0671619..fdbd0ece0 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -235,7 +235,7 @@ def __init__(self, framework: ops.Framework): dispatch.chmod(0o755) with patch.dict(os.environ, fake_environ): - with patch('ops._main._emit_charm_event') as mock_charm_event: + with patch('ops.main._emit_charm_event') as mock_charm_event: ops.main(MyCharm) assert mock_charm_event.call_count == 1 From 2678252ae6f612de714d65b2637c46eeddce5604 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 16:52:05 +0900 Subject: [PATCH 09/33] test: split up #1331 tests --- test/test_main.py | 2 +- ...invocations.py => test_main_invocation.py} | 97 +++++-------------- test/test_main_type_hint.py | 78 +++++++++++++++ 3 files changed, 105 insertions(+), 72 deletions(-) rename test/{test_main_invocations.py => test_main_invocation.py} (50%) create mode 100644 test/test_main_type_hint.py diff --git a/test/test_main.py b/test/test_main.py index fdbd0ece0..3a0671619 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -235,7 +235,7 @@ def __init__(self, framework: ops.Framework): dispatch.chmod(0o755) with patch.dict(os.environ, fake_environ): - with patch('ops.main._emit_charm_event') as mock_charm_event: + with patch('ops._main._emit_charm_event') as mock_charm_event: ops.main(MyCharm) assert mock_charm_event.call_count == 1 diff --git a/test/test_main_invocations.py b/test/test_main_invocation.py similarity index 50% rename from test/test_main_invocations.py rename to test/test_main_invocation.py index 73805402b..9119c2da1 100644 --- a/test/test_main_invocations.py +++ b/test/test_main_invocation.py @@ -11,53 +11,17 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -""" -Verify that `ops.main` can be invoked in every possible way. - -Validate that main can be called at runtime and according to the static type checker. - -Runtime tests: -- Ensure that `ops.main` and `ops.main.main` can be invoked with a charm class. -- Across 3 import styles: ops, main from ops, main from ops.main. -- Confirm that calling main without a charm class fails. - -Typing tests: -- Ensure that `ops.main` and `ops.main.main` are callable with correct argument. -- Across same 3 import styles as above. -- Confirm that calling main without a charm class is caught by static analysis. -""" - import os from pathlib import Path -from typing import Callable, Type from unittest.mock import Mock import pytest import ops -Reset = Callable[[], None] - - -def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): - """Usage: - from somewhere import main - type_test_dummy(main) - """ - - -def type_test_negative(_arg: Callable[[], None]): - """Usage: - from somewhere import main - type_test_negative(main) # type: ignore - - The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, - should the passed argument match the expected argument type. - """ - @pytest.fixture -def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): +def charm_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): monkeypatch.setattr('sys.argv', ('hooks/install',)) monkeypatch.setattr('ops._main._emit_charm_event', Mock()) monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock()) @@ -70,9 +34,9 @@ def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): (tmp_path / 'metadata.yaml').write_text('name: test', encoding='utf-8') (tmp_path / 'dispatch').absolute().touch(mode=0o755) - yield (reset := lambda: os.environ.pop('OPERATOR_DISPATCH', None)) + yield - reset() + os.environ.pop('OPERATOR_DISPATCH', None) class IdleCharm(ops.CharmBase): @@ -80,72 +44,63 @@ def __init__(self, framework: ops.Framework): super().__init__(framework) -def test_top_level_import(reset: Reset): +def test_top_level_import(charm_env: None): import ops - type_test_dummy(ops.main.__call__) # pyright is quirky - type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore - type_test_negative(ops.main.main) # type: ignore - ops.main(IdleCharm) - reset() - ops.main.main(IdleCharm) - with pytest.raises(TypeError): ops.main() # type: ignore + +def test_top_level_import_legacy_call(charm_env: None): + import ops + + ops.main.main(IdleCharm) + with pytest.raises(TypeError): ops.main.main() # type: ignore -def test_submodule_import(reset: Reset): +def test_submodule_import(charm_env: None): import ops.main - type_test_dummy(ops.main.__call__) # type: ignore FIXME - type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore - type_test_negative(ops.main.main) # type: ignore - - ops.main(IdleCharm) # type: ignore FIXME - - reset() - ops.main.main(IdleCharm) + ops.main(IdleCharm) # type: ignore https://github.com/microsoft/pyright/issues/8830 with pytest.raises(TypeError): ops.main() # type: ignore + +def test_submodule_import_legacy_call(charm_env: None): + import ops.main + + ops.main.main(IdleCharm) + with pytest.raises(TypeError): ops.main.main() # type: ignore -def test_import_from_top_level_module(reset: Reset): +def test_import_from_top_level_module(charm_env: None): from ops import main - type_test_dummy(main.__call__) - type_test_dummy(main.main) - type_test_negative(main.__call__) # type: ignore - type_test_negative(main.main) # type: ignore - main(IdleCharm) - reset() - main.main(IdleCharm) - with pytest.raises(TypeError): main() # type: ignore + +def test_import_from_top_level_module_legacy_call(charm_env: None): + from ops import main + + main.main(IdleCharm) + with pytest.raises(TypeError): main.main() # type: ignore -def test_import_from_submodule(reset: Reset): +def test_legacy_import_from_submodule(charm_env: None): from ops.main import main - type_test_dummy(main) - type_test_negative(main) # type: ignore - main(IdleCharm) with pytest.raises(TypeError): diff --git a/test/test_main_type_hint.py b/test/test_main_type_hint.py new file mode 100644 index 000000000..b23e22c37 --- /dev/null +++ b/test/test_main_type_hint.py @@ -0,0 +1,78 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from typing import Callable, Type + +import ops + + +def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): + """ + Helper to verify that + Usage: + + from somewhere import main + + type_test_dummy(main) + """ + + +def type_test_negative(_arg: Callable[[], None]): + """ + Usage: + + from somewhere import main + + type_test_negative(main) # type: ignore + + The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, + should the passed argument match the expected argument type. + """ + + +def top_level_import(): + import ops + + type_test_dummy(ops.main.__call__) # pyright is quirky + type_test_dummy(ops.main.main) + + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + +def submodule_import(): + import ops.main + + type_test_dummy(ops.main.__call__) # type: ignore https://github.com/microsoft/pyright/issues/8830 + type_test_dummy(ops.main.main) + + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + +def import_from_top_level_module(): + from ops import main + + type_test_dummy(main.__call__) + type_test_dummy(main.main) + + type_test_negative(main.__call__) # type: ignore + type_test_negative(main.main) # type: ignore + + +def import_from_submodule(): + from ops.main import main + + type_test_dummy(main) + + type_test_negative(main) # type: ignore From aaf22d19646a6812b1b8bd33a814e164821c14ce Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 22 Aug 2024 15:58:59 +0900 Subject: [PATCH 10/33] fix: rework ops.main type hints to allow different import flavours --- test/test_main_invocations.py | 136 ++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 test/test_main_invocations.py diff --git a/test/test_main_invocations.py b/test/test_main_invocations.py new file mode 100644 index 000000000..239035cf1 --- /dev/null +++ b/test/test_main_invocations.py @@ -0,0 +1,136 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os +from pathlib import Path +from typing import Callable, Type +from unittest.mock import Mock + +import pytest + +import ops + +Reset = Callable[[], None] + + +def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): + """Usage: + from somewhere import main + type_test_dummy(main) + """ + + +def type_test_negative(_arg: Callable[[], None]): + """Usage: + from somewhere import main + type_test_negative(main) # type: ignore + + The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, + should the passed argument match the expected argument type. + """ + + +@pytest.fixture +def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + monkeypatch.setattr('sys.argv', ('hooks/install',)) + monkeypatch.setattr('ops._main._emit_charm_event', Mock()) + monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock()) + monkeypatch.setattr('ops.charm._evaluate_status', Mock()) + monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path)) + monkeypatch.setenv('JUJU_UNIT_NAME', 'test_main/0') + monkeypatch.setenv('JUJU_MODEL_NAME', 'mymodel') + monkeypatch.setenv('JUJU_DISPATCH_PATH', 'hooks/install') + monkeypatch.setenv('JUJU_VERSION', '3.5.0') + (tmp_path / 'metadata.yaml').write_text('name: test', encoding='utf-8') + (tmp_path / 'dispatch').absolute().touch(mode=0o755) + + yield (reset := lambda: os.environ.pop('OPERATOR_DISPATCH', None)) + + reset() + + +class IdleCharm(ops.CharmBase): + def __init__(self, framework: ops.Framework): + super().__init__(framework) + + +def test_top_level_import(reset: Reset): + import ops + + type_test_dummy(ops.main.__call__) # pyright is quirky + type_test_dummy(ops.main.main) + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + ops.main(IdleCharm) + + reset() + ops.main.main(IdleCharm) + + with pytest.raises(TypeError): + ops.main() # type: ignore + + with pytest.raises(TypeError): + ops.main.main() # type: ignore + + +def test_submodule_import(reset: Reset): + import ops.main + + type_test_dummy(ops.main.__call__) # type: ignore FIXME + type_test_dummy(ops.main.main) + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + ops.main(IdleCharm) # type: ignore FIXME + + reset() + ops.main.main(IdleCharm) + + with pytest.raises(TypeError): + ops.main() # type: ignore + + with pytest.raises(TypeError): + ops.main.main() # type: ignore + + +def test_import_from_top_level_module(reset: Reset): + from ops import main + + type_test_dummy(main.__call__) + type_test_dummy(main.main) + type_test_negative(main.__call__) # type: ignore + type_test_negative(main.main) # type: ignore + + main(IdleCharm) + + reset() + main.main(IdleCharm) + + with pytest.raises(TypeError): + main() # type: ignore + + with pytest.raises(TypeError): + main.main() # type: ignore + + +def test_import_from_submodule(reset: Reset): + from ops.main import main + + type_test_dummy(main) + type_test_negative(main) # type: ignore + + main(IdleCharm) + + with pytest.raises(TypeError): + main() # type: ignore From 0120397c4e4fee041110b0e2325b4409337fd878 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 14:21:35 +0900 Subject: [PATCH 11/33] add test modile docstring --- test/test_main_invocations.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/test_main_invocations.py b/test/test_main_invocations.py index 239035cf1..73805402b 100644 --- a/test/test_main_invocations.py +++ b/test/test_main_invocations.py @@ -11,6 +11,22 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +""" +Verify that `ops.main` can be invoked in every possible way. + +Validate that main can be called at runtime and according to the static type checker. + +Runtime tests: +- Ensure that `ops.main` and `ops.main.main` can be invoked with a charm class. +- Across 3 import styles: ops, main from ops, main from ops.main. +- Confirm that calling main without a charm class fails. + +Typing tests: +- Ensure that `ops.main` and `ops.main.main` are callable with correct argument. +- Across same 3 import styles as above. +- Confirm that calling main without a charm class is caught by static analysis. +""" + import os from pathlib import Path from typing import Callable, Type From 54a333060fa0eb57ea4d0ec5c773706851be9b6c Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 16:52:05 +0900 Subject: [PATCH 12/33] test: split up #1331 tests --- test/test_main_invocations.py | 152 ---------------------------------- 1 file changed, 152 deletions(-) delete mode 100644 test/test_main_invocations.py diff --git a/test/test_main_invocations.py b/test/test_main_invocations.py deleted file mode 100644 index 73805402b..000000000 --- a/test/test_main_invocations.py +++ /dev/null @@ -1,152 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -""" -Verify that `ops.main` can be invoked in every possible way. - -Validate that main can be called at runtime and according to the static type checker. - -Runtime tests: -- Ensure that `ops.main` and `ops.main.main` can be invoked with a charm class. -- Across 3 import styles: ops, main from ops, main from ops.main. -- Confirm that calling main without a charm class fails. - -Typing tests: -- Ensure that `ops.main` and `ops.main.main` are callable with correct argument. -- Across same 3 import styles as above. -- Confirm that calling main without a charm class is caught by static analysis. -""" - -import os -from pathlib import Path -from typing import Callable, Type -from unittest.mock import Mock - -import pytest - -import ops - -Reset = Callable[[], None] - - -def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): - """Usage: - from somewhere import main - type_test_dummy(main) - """ - - -def type_test_negative(_arg: Callable[[], None]): - """Usage: - from somewhere import main - type_test_negative(main) # type: ignore - - The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, - should the passed argument match the expected argument type. - """ - - -@pytest.fixture -def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): - monkeypatch.setattr('sys.argv', ('hooks/install',)) - monkeypatch.setattr('ops._main._emit_charm_event', Mock()) - monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock()) - monkeypatch.setattr('ops.charm._evaluate_status', Mock()) - monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path)) - monkeypatch.setenv('JUJU_UNIT_NAME', 'test_main/0') - monkeypatch.setenv('JUJU_MODEL_NAME', 'mymodel') - monkeypatch.setenv('JUJU_DISPATCH_PATH', 'hooks/install') - monkeypatch.setenv('JUJU_VERSION', '3.5.0') - (tmp_path / 'metadata.yaml').write_text('name: test', encoding='utf-8') - (tmp_path / 'dispatch').absolute().touch(mode=0o755) - - yield (reset := lambda: os.environ.pop('OPERATOR_DISPATCH', None)) - - reset() - - -class IdleCharm(ops.CharmBase): - def __init__(self, framework: ops.Framework): - super().__init__(framework) - - -def test_top_level_import(reset: Reset): - import ops - - type_test_dummy(ops.main.__call__) # pyright is quirky - type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore - type_test_negative(ops.main.main) # type: ignore - - ops.main(IdleCharm) - - reset() - ops.main.main(IdleCharm) - - with pytest.raises(TypeError): - ops.main() # type: ignore - - with pytest.raises(TypeError): - ops.main.main() # type: ignore - - -def test_submodule_import(reset: Reset): - import ops.main - - type_test_dummy(ops.main.__call__) # type: ignore FIXME - type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore - type_test_negative(ops.main.main) # type: ignore - - ops.main(IdleCharm) # type: ignore FIXME - - reset() - ops.main.main(IdleCharm) - - with pytest.raises(TypeError): - ops.main() # type: ignore - - with pytest.raises(TypeError): - ops.main.main() # type: ignore - - -def test_import_from_top_level_module(reset: Reset): - from ops import main - - type_test_dummy(main.__call__) - type_test_dummy(main.main) - type_test_negative(main.__call__) # type: ignore - type_test_negative(main.main) # type: ignore - - main(IdleCharm) - - reset() - main.main(IdleCharm) - - with pytest.raises(TypeError): - main() # type: ignore - - with pytest.raises(TypeError): - main.main() # type: ignore - - -def test_import_from_submodule(reset: Reset): - from ops.main import main - - type_test_dummy(main) - type_test_negative(main) # type: ignore - - main(IdleCharm) - - with pytest.raises(TypeError): - main() # type: ignore From 2ae8a1ce711ec6f4b7067f4669ccffaaa214be84 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 17:51:38 +0900 Subject: [PATCH 13/33] chore: deprecate ops.main.main() --- ops/{main.py => _main.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename ops/{main.py => _main.py} (100%) diff --git a/ops/main.py b/ops/_main.py similarity index 100% rename from ops/main.py rename to ops/_main.py From b80fc1a6f0d53bebd6907d16522239ab48206e26 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 17:52:16 +0900 Subject: [PATCH 14/33] chore: deprecate ops.main.main() pt 2 --- ops/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index 2d160cb32..ea94ed123 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -182,7 +182,7 @@ # import was here previously from . import charm # type: ignore # noqa: F401 `.charm` imported but unused -from . import main as _main +from ._main import main # Explicitly import names from submodules so users can just "import ops" and # then use them as "ops.X". @@ -319,6 +319,3 @@ # rather than a runtime concern. from .version import version as __version__ - -# This allows "import ops; ops.main(Charm)" to work as expected. -main = _main._CallableMainModule('ops.main', _main.__doc__) From ab338bbcf1ac5f117a95bf06a786de01bbe43125 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 27 Aug 2024 09:13:01 +0900 Subject: [PATCH 15/33] chore: deprecate ops.main.main() pt 3 --- ops/_main.py | 3 +++ ops/main.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 ops/main.py diff --git a/ops/_main.py b/ops/_main.py index a255fc66b..def538fb1 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -556,6 +556,9 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[ sys.exit(e.exit_code) +# FIXME maybe support the old-school import ops; ops.main.main() +main.main = main + # Support old and new style main calls at run time and for type checking # - ops.main.main(SomeCharm) # - ops.main(SomeCharm) diff --git a/ops/main.py b/ops/main.py new file mode 100644 index 000000000..7a35e82d1 --- /dev/null +++ b/ops/main.py @@ -0,0 +1,47 @@ +# Copyright 2020 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Support legacy ops.main.main() import.""" + +import warnings +from typing import Optional, Type + +import ops.charm + +from . import _main + + +def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None): + """Legacy entrypoint to set up the charm and dispatch the observed event. + + The event name is based on the way this executable was called (argv[0]). + + .. jujuremoved:: 4.0 + The ``use_juju_for_storage`` argument is not available from Juju 4.0 + + Args: + charm_class: the charm class to instantiate and receive the event. + use_juju_for_storage: whether to use controller-side storage. If not specified + then Kubernetes charms that haven't previously used local storage and that + are running on a new enough Juju default to controller-side storage, + otherwise local storage is used. + + .. deprecated:: 2.16.0 + This entrypoint has been deprecated, use `ops.main()` instead. + """ + warnings.warn( + 'Calling `ops.main.main()` is deprecated, call `ops.main()` instead', + DeprecationWarning, + stacklevel=2, + ) + return _main.main(charm_class=charm_class, use_juju_for_storage=use_juju_for_storage) From 8d65e7f3f3e4f9647ff4cbe2c1a132b2c5a3b11d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 27 Aug 2024 10:02:33 +0900 Subject: [PATCH 16/33] wip: Protocol --- ops/_main.py | 12 +++++++++++- test/test_main_invocation.py | 2 +- test/test_main_type_hint.py | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/ops/_main.py b/ops/_main.py index def538fb1..f31f3dafa 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -27,7 +27,7 @@ import warnings from pathlib import Path from types import ModuleType -from typing import Any, Dict, List, Optional, Tuple, Type, Union, cast +from typing import Any, Callable, Dict, List, Optional, Protocol, Tuple, Type, Union, cast import ops.charm import ops.framework @@ -557,8 +557,18 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[ # FIXME maybe support the old-school import ops; ops.main.main() +class Main(Protocol): + def __call__( + self, charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None + ): ... + + main: Callable[[Type[ops.charm.CharmBase], Optional[bool]], None] + + +main: Main = main # type: ignore main.main = main + # Support old and new style main calls at run time and for type checking # - ops.main.main(SomeCharm) # - ops.main(SomeCharm) diff --git a/test/test_main_invocation.py b/test/test_main_invocation.py index 9119c2da1..598664b08 100644 --- a/test/test_main_invocation.py +++ b/test/test_main_invocation.py @@ -65,7 +65,7 @@ def test_top_level_import_legacy_call(charm_env: None): def test_submodule_import(charm_env: None): import ops.main - ops.main(IdleCharm) # type: ignore https://github.com/microsoft/pyright/issues/8830 + ops.main(IdleCharm) # type: ignore # https://github.com/microsoft/pyright/issues/8830 with pytest.raises(TypeError): ops.main() # type: ignore diff --git a/test/test_main_type_hint.py b/test/test_main_type_hint.py index b23e22c37..f52eb8581 100644 --- a/test/test_main_type_hint.py +++ b/test/test_main_type_hint.py @@ -53,7 +53,7 @@ def top_level_import(): def submodule_import(): import ops.main - type_test_dummy(ops.main.__call__) # type: ignore https://github.com/microsoft/pyright/issues/8830 + type_test_dummy(ops.main.__call__) # type: ignore # https://github.com/microsoft/pyright/issues/8830 type_test_dummy(ops.main.main) type_test_negative(ops.main.__call__) # type: ignore From 5893b5e9e8c6514e9cdbadc7213a416000e47034 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 06:36:06 +0900 Subject: [PATCH 17/33] =?UTF-8?q?polish=F0=9F=A7=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ops/__init__.py | 20 ++++++++++++++++++-- ops/_main.py | 23 +---------------------- test/test_main.py | 2 +- test/test_main_invocation.py | 12 +++++++++--- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index ea94ed123..da8c2a325 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -173,6 +173,7 @@ # The isort command wants to rearrange the nicely-formatted imports below; # just skip it for this file. # isort:skip_file +from typing import Optional, Protocol, Type, cast # Import pebble explicitly. It's the one module we don't import names from below. from . import pebble @@ -180,9 +181,10 @@ # Also import charm explicitly. This is not strictly necessary as the # "from .charm" import automatically does that, but be explicit since this # import was here previously -from . import charm # type: ignore # noqa: F401 `.charm` imported but unused +from . import charm -from ._main import main +from . import _main +from . import main as _legacy_main # Explicitly import names from submodules so users can just "import ops" and # then use them as "ops.X". @@ -319,3 +321,17 @@ # rather than a runtime concern. from .version import version as __version__ + + +class Main(Protocol): + def __call__( + self, charm_class: Type[charm.CharmBase], use_juju_for_storage: Optional[bool] = None + ): ... + + def main( + self, charm_class: Type[charm.CharmBase], use_juju_for_storage: Optional[bool] = None + ): ... + + +main: Main = cast(Main, _main.main) +main.main = _legacy_main.main diff --git a/ops/_main.py b/ops/_main.py index f31f3dafa..6934235ea 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -26,8 +26,7 @@ import sys import warnings from pathlib import Path -from types import ModuleType -from typing import Any, Callable, Dict, List, Optional, Protocol, Tuple, Type, Union, cast +from typing import Any, Dict, List, Optional, Tuple, Type, Union, cast import ops.charm import ops.framework @@ -554,23 +553,3 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[ manager.run() except _Abort as e: sys.exit(e.exit_code) - - -# FIXME maybe support the old-school import ops; ops.main.main() -class Main(Protocol): - def __call__( - self, charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None - ): ... - - main: Callable[[Type[ops.charm.CharmBase], Optional[bool]], None] - - -main: Main = main # type: ignore -main.main = main - - -# Support old and new style main calls at run time and for type checking -# - ops.main.main(SomeCharm) -# - ops.main(SomeCharm) -class _CallableMainModule(ModuleType): # pyright: ignore[reportUnusedClass] as it's used in __init__.py - __call__ = main = staticmethod(main) diff --git a/test/test_main.py b/test/test_main.py index 3a0671619..6aea9eed4 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -30,8 +30,8 @@ import pytest import ops +from ops._main import _should_use_controller_storage from ops.jujucontext import _JujuContext -from ops.main import _should_use_controller_storage from ops.storage import SQLiteStorage from .charms.test_main.src.charm import MyCharmEvents diff --git a/test/test_main_invocation.py b/test/test_main_invocation.py index 598664b08..4e1b5e73b 100644 --- a/test/test_main_invocation.py +++ b/test/test_main_invocation.py @@ -58,6 +58,9 @@ def test_top_level_import_legacy_call(charm_env: None): ops.main.main(IdleCharm) + with pytest.deprecated_call(): + ops.main.main(IdleCharm) + with pytest.raises(TypeError): ops.main.main() # type: ignore @@ -74,7 +77,8 @@ def test_submodule_import(charm_env: None): def test_submodule_import_legacy_call(charm_env: None): import ops.main - ops.main.main(IdleCharm) + with pytest.deprecated_call(): + ops.main.main(IdleCharm) with pytest.raises(TypeError): ops.main.main() # type: ignore @@ -92,7 +96,8 @@ def test_import_from_top_level_module(charm_env: None): def test_import_from_top_level_module_legacy_call(charm_env: None): from ops import main - main.main(IdleCharm) + with pytest.deprecated_call(): + main.main(IdleCharm) with pytest.raises(TypeError): main.main() # type: ignore @@ -101,7 +106,8 @@ def test_import_from_top_level_module_legacy_call(charm_env: None): def test_legacy_import_from_submodule(charm_env: None): from ops.main import main - main(IdleCharm) + with pytest.deprecated_call(): + main(IdleCharm) with pytest.raises(TypeError): main() # type: ignore From 173981f7641bde1da79926539ec5249a66443e66 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 06:37:19 +0900 Subject: [PATCH 18/33] =?UTF-8?q?polish=F0=9F=A7=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ops/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ops/__init__.py b/ops/__init__.py index da8c2a325..b4ddb25e8 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -173,7 +173,7 @@ # The isort command wants to rearrange the nicely-formatted imports below; # just skip it for this file. # isort:skip_file -from typing import Optional, Protocol, Type, cast +from typing import Optional, Protocol, Type, cast # Import pebble explicitly. It's the one module we don't import names from below. from . import pebble From 627b96d5fdcc7bc948e4f46d9c589357fa11f67c Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 06:43:18 +0900 Subject: [PATCH 19/33] =?UTF-8?q?polish=F0=9F=A7=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/test_main_invocation.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_main_invocation.py b/test/test_main_invocation.py index 4e1b5e73b..40f9c97bd 100644 --- a/test/test_main_invocation.py +++ b/test/test_main_invocation.py @@ -56,8 +56,6 @@ def test_top_level_import(charm_env: None): def test_top_level_import_legacy_call(charm_env: None): import ops - ops.main.main(IdleCharm) - with pytest.deprecated_call(): ops.main.main(IdleCharm) From 877897e1f24eb84adf5b862c772437647eace8d6 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 22 Aug 2024 15:58:59 +0900 Subject: [PATCH 20/33] fix: rework ops.main type hints to allow different import flavours --- test/test_main_invocations.py | 136 ++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 test/test_main_invocations.py diff --git a/test/test_main_invocations.py b/test/test_main_invocations.py new file mode 100644 index 000000000..239035cf1 --- /dev/null +++ b/test/test_main_invocations.py @@ -0,0 +1,136 @@ +# Copyright 2024 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os +from pathlib import Path +from typing import Callable, Type +from unittest.mock import Mock + +import pytest + +import ops + +Reset = Callable[[], None] + + +def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): + """Usage: + from somewhere import main + type_test_dummy(main) + """ + + +def type_test_negative(_arg: Callable[[], None]): + """Usage: + from somewhere import main + type_test_negative(main) # type: ignore + + The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, + should the passed argument match the expected argument type. + """ + + +@pytest.fixture +def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + monkeypatch.setattr('sys.argv', ('hooks/install',)) + monkeypatch.setattr('ops._main._emit_charm_event', Mock()) + monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock()) + monkeypatch.setattr('ops.charm._evaluate_status', Mock()) + monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path)) + monkeypatch.setenv('JUJU_UNIT_NAME', 'test_main/0') + monkeypatch.setenv('JUJU_MODEL_NAME', 'mymodel') + monkeypatch.setenv('JUJU_DISPATCH_PATH', 'hooks/install') + monkeypatch.setenv('JUJU_VERSION', '3.5.0') + (tmp_path / 'metadata.yaml').write_text('name: test', encoding='utf-8') + (tmp_path / 'dispatch').absolute().touch(mode=0o755) + + yield (reset := lambda: os.environ.pop('OPERATOR_DISPATCH', None)) + + reset() + + +class IdleCharm(ops.CharmBase): + def __init__(self, framework: ops.Framework): + super().__init__(framework) + + +def test_top_level_import(reset: Reset): + import ops + + type_test_dummy(ops.main.__call__) # pyright is quirky + type_test_dummy(ops.main.main) + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + ops.main(IdleCharm) + + reset() + ops.main.main(IdleCharm) + + with pytest.raises(TypeError): + ops.main() # type: ignore + + with pytest.raises(TypeError): + ops.main.main() # type: ignore + + +def test_submodule_import(reset: Reset): + import ops.main + + type_test_dummy(ops.main.__call__) # type: ignore FIXME + type_test_dummy(ops.main.main) + type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main.main) # type: ignore + + ops.main(IdleCharm) # type: ignore FIXME + + reset() + ops.main.main(IdleCharm) + + with pytest.raises(TypeError): + ops.main() # type: ignore + + with pytest.raises(TypeError): + ops.main.main() # type: ignore + + +def test_import_from_top_level_module(reset: Reset): + from ops import main + + type_test_dummy(main.__call__) + type_test_dummy(main.main) + type_test_negative(main.__call__) # type: ignore + type_test_negative(main.main) # type: ignore + + main(IdleCharm) + + reset() + main.main(IdleCharm) + + with pytest.raises(TypeError): + main() # type: ignore + + with pytest.raises(TypeError): + main.main() # type: ignore + + +def test_import_from_submodule(reset: Reset): + from ops.main import main + + type_test_dummy(main) + type_test_negative(main) # type: ignore + + main(IdleCharm) + + with pytest.raises(TypeError): + main() # type: ignore From c8636158b7447f8055506d807bc16daae977e769 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 26 Aug 2024 16:52:05 +0900 Subject: [PATCH 21/33] test: split up #1331 tests --- test/test_main_invocations.py | 136 ---------------------------------- 1 file changed, 136 deletions(-) delete mode 100644 test/test_main_invocations.py diff --git a/test/test_main_invocations.py b/test/test_main_invocations.py deleted file mode 100644 index 239035cf1..000000000 --- a/test/test_main_invocations.py +++ /dev/null @@ -1,136 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import os -from pathlib import Path -from typing import Callable, Type -from unittest.mock import Mock - -import pytest - -import ops - -Reset = Callable[[], None] - - -def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): - """Usage: - from somewhere import main - type_test_dummy(main) - """ - - -def type_test_negative(_arg: Callable[[], None]): - """Usage: - from somewhere import main - type_test_negative(main) # type: ignore - - The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, - should the passed argument match the expected argument type. - """ - - -@pytest.fixture -def reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): - monkeypatch.setattr('sys.argv', ('hooks/install',)) - monkeypatch.setattr('ops._main._emit_charm_event', Mock()) - monkeypatch.setattr('ops._main._Manager._setup_root_logging', Mock()) - monkeypatch.setattr('ops.charm._evaluate_status', Mock()) - monkeypatch.setenv('JUJU_CHARM_DIR', str(tmp_path)) - monkeypatch.setenv('JUJU_UNIT_NAME', 'test_main/0') - monkeypatch.setenv('JUJU_MODEL_NAME', 'mymodel') - monkeypatch.setenv('JUJU_DISPATCH_PATH', 'hooks/install') - monkeypatch.setenv('JUJU_VERSION', '3.5.0') - (tmp_path / 'metadata.yaml').write_text('name: test', encoding='utf-8') - (tmp_path / 'dispatch').absolute().touch(mode=0o755) - - yield (reset := lambda: os.environ.pop('OPERATOR_DISPATCH', None)) - - reset() - - -class IdleCharm(ops.CharmBase): - def __init__(self, framework: ops.Framework): - super().__init__(framework) - - -def test_top_level_import(reset: Reset): - import ops - - type_test_dummy(ops.main.__call__) # pyright is quirky - type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore - type_test_negative(ops.main.main) # type: ignore - - ops.main(IdleCharm) - - reset() - ops.main.main(IdleCharm) - - with pytest.raises(TypeError): - ops.main() # type: ignore - - with pytest.raises(TypeError): - ops.main.main() # type: ignore - - -def test_submodule_import(reset: Reset): - import ops.main - - type_test_dummy(ops.main.__call__) # type: ignore FIXME - type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore - type_test_negative(ops.main.main) # type: ignore - - ops.main(IdleCharm) # type: ignore FIXME - - reset() - ops.main.main(IdleCharm) - - with pytest.raises(TypeError): - ops.main() # type: ignore - - with pytest.raises(TypeError): - ops.main.main() # type: ignore - - -def test_import_from_top_level_module(reset: Reset): - from ops import main - - type_test_dummy(main.__call__) - type_test_dummy(main.main) - type_test_negative(main.__call__) # type: ignore - type_test_negative(main.main) # type: ignore - - main(IdleCharm) - - reset() - main.main(IdleCharm) - - with pytest.raises(TypeError): - main() # type: ignore - - with pytest.raises(TypeError): - main.main() # type: ignore - - -def test_import_from_submodule(reset: Reset): - from ops.main import main - - type_test_dummy(main) - type_test_negative(main) # type: ignore - - main(IdleCharm) - - with pytest.raises(TypeError): - main() # type: ignore From f3c6359dab165aad46f9a610e195494fe6945fe3 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 06:42:52 +0900 Subject: [PATCH 22/33] use a plain class --- ops/__init__.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index b4ddb25e8..3fca2e288 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -173,7 +173,7 @@ # The isort command wants to rearrange the nicely-formatted imports below; # just skip it for this file. # isort:skip_file -from typing import Optional, Protocol, Type, cast +from typing import Optional, Type # Import pebble explicitly. It's the one module we don't import names from below. from . import pebble @@ -323,15 +323,18 @@ from .version import version as __version__ -class Main(Protocol): +class Main: def __call__( self, charm_class: Type[charm.CharmBase], use_juju_for_storage: Optional[bool] = None - ): ... + ): + return _main.main(charm_class=charm_class, use_juju_for_storage=use_juju_for_storage) def main( self, charm_class: Type[charm.CharmBase], use_juju_for_storage: Optional[bool] = None - ): ... + ): + return _legacy_main.main( + charm_class=charm_class, use_juju_for_storage=use_juju_for_storage + ) -main: Main = cast(Main, _main.main) -main.main = _legacy_main.main +main = Main() From bac26ca13664cee444ba46f153e0b8eeafb6a394 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 10:30:25 +0900 Subject: [PATCH 23/33] Update the docs --- docs/index.rst | 19 +++++++++++++++++-- ops/__init__.py | 30 ++++++++++++++++++++++++++++-- ops/_main.py | 19 ++----------------- ops/main.py | 14 ++------------ 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 390398349..4e3527d7c 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -10,12 +10,27 @@ ops module ========== .. automodule:: ops + :exclude-members: main -main ------------ +ops.main entry point +==================== +.. autofunction:: ops.main + :noindex: + + +_main module +------------ + +.. automodule:: ops._main + :noindex: + + +legacy main module +------------------ .. automodule:: ops.main + :noindex: ops.pebble module diff --git a/ops/__init__.py b/ops/__init__.py index 3fca2e288..82428c917 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -323,7 +323,7 @@ from .version import version as __version__ -class Main: +class _Main: def __call__( self, charm_class: Type[charm.CharmBase], use_juju_for_storage: Optional[bool] = None ): @@ -337,4 +337,30 @@ def main( ) -main = Main() +main = _Main() +"""Set up the charm and dispatch the observed event. + +The event name is based on the way this executable was called (argv[0]). + +.. jujuremoved:: 4.0 + The ``use_juju_for_storage`` argument is not available from Juju 4.0 + +Args: + charm_class: the charm class to instantiate and receive the event. + use_juju_for_storage: whether to use controller-side storage. If not specified + then Kubernetes charms that haven't previously used local storage and that + are running on a new enough Juju default to controller-side storage, + otherwise local storage is used. + +Recommended usage: + +.. code-block:: python + + import ops + + + class SomeCharm(ops.CharmBase): + ... + + ops.main(SomeCharm) +""" diff --git a/ops/_main.py b/ops/_main.py index 6934235ea..2b3ad0671 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -12,12 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Main entry point to the framework. - -Note that this module is callable, and calls the :func:`ops.main.main` function. -This is so that :code:`import ops` followed by :code:`ops.main(MyCharm)` works -as expected. -""" +"""Imeplement the main entry point to the framework.""" import logging import os @@ -535,17 +530,7 @@ def run(self): def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None): """Set up the charm and dispatch the observed event. - The event name is based on the way this executable was called (argv[0]). - - .. jujuremoved:: 4.0 - The ``use_juju_for_storage`` argument is not available from Juju 4.0 - - Args: - charm_class: the charm class to instantiate and receive the event. - use_juju_for_storage: whether to use controller-side storage. If not specified - then Kubernetes charms that haven't previously used local storage and that - are running on a new enough Juju default to controller-side storage, - otherwise local storage is used. + See :func:`ops.main` for details. """ try: manager = _Manager(charm_class, use_juju_for_storage=use_juju_for_storage) diff --git a/ops/main.py b/ops/main.py index 7a35e82d1..dc2c88668 100644 --- a/ops/main.py +++ b/ops/main.py @@ -24,20 +24,10 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None): """Legacy entrypoint to set up the charm and dispatch the observed event. - The event name is based on the way this executable was called (argv[0]). - - .. jujuremoved:: 4.0 - The ``use_juju_for_storage`` argument is not available from Juju 4.0 - - Args: - charm_class: the charm class to instantiate and receive the event. - use_juju_for_storage: whether to use controller-side storage. If not specified - then Kubernetes charms that haven't previously used local storage and that - are running on a new enough Juju default to controller-side storage, - otherwise local storage is used. - .. deprecated:: 2.16.0 This entrypoint has been deprecated, use `ops.main()` instead. + + See :func:`ops.main` for details. """ warnings.warn( 'Calling `ops.main.main()` is deprecated, call `ops.main()` instead', From 7f58996c4235cdbdf1fb08b30f8a14653b2c4422 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 11:24:53 +0900 Subject: [PATCH 24/33] docstring fixes --- ops/_main.py | 2 +- test/test_main_type_hint.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ops/_main.py b/ops/_main.py index 2b3ad0671..db7f16666 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Imeplement the main entry point to the framework.""" +"""Implement the main entry point to the framework.""" import logging import os diff --git a/test/test_main_type_hint.py b/test/test_main_type_hint.py index f52eb8581..7617eb695 100644 --- a/test/test_main_type_hint.py +++ b/test/test_main_type_hint.py @@ -18,7 +18,7 @@ def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): """ - Helper to verify that + Helper to verify the function signature of ops.main and ops.main.main Usage: from somewhere import main @@ -29,6 +29,7 @@ def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): def type_test_negative(_arg: Callable[[], None]): """ + Helper for negative tests of the function signatures of ops.main and ops.main.main Usage: from somewhere import main From e8b253ba87688027ed13d55903e0a38ad8f0c5e0 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 11:30:26 +0900 Subject: [PATCH 25/33] plug doc build issue for now --- ops/_main.py | 2 +- ops/main.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ops/_main.py b/ops/_main.py index db7f16666..ff09cbc7e 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -530,7 +530,7 @@ def run(self): def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None): """Set up the charm and dispatch the observed event. - See :func:`ops.main` for details. + See ``ops.main`` for details. """ try: manager = _Manager(charm_class, use_juju_for_storage=use_juju_for_storage) diff --git a/ops/main.py b/ops/main.py index dc2c88668..032288bb6 100644 --- a/ops/main.py +++ b/ops/main.py @@ -27,7 +27,7 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[ .. deprecated:: 2.16.0 This entrypoint has been deprecated, use `ops.main()` instead. - See :func:`ops.main` for details. + See ``ops.main()`` for details. """ warnings.warn( 'Calling `ops.main.main()` is deprecated, call `ops.main()` instead', From 6113684d1345dd2f3d5b251a0d9540e80891dd55 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 14:02:48 +0900 Subject: [PATCH 26/33] address review comments --- docs/index.rst | 8 -------- ops/__init__.py | 7 ++++--- ops/_main.py | 2 +- ops/main.py | 2 +- test/test_main_invocation.py | 19 +++++++------------ test/test_main_type_hint.py | 12 ++++++------ 6 files changed, 19 insertions(+), 31 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 4e3527d7c..af4de65ca 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -16,14 +16,6 @@ ops module ops.main entry point ==================== .. autofunction:: ops.main - :noindex: - - -_main module ------------- - -.. automodule:: ops._main - :noindex: legacy main module diff --git a/ops/__init__.py b/ops/__init__.py index 82428c917..05d153dcc 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -347,10 +347,11 @@ def main( Args: charm_class: the charm class to instantiate and receive the event. - use_juju_for_storage: whether to use controller-side storage. If not specified - then Kubernetes charms that haven't previously used local storage and that + use_juju_for_storage: whether to use controller-side storage. + The default is ``False`` for most charms. + Podspec charms that haven't previously used local storage and that are running on a new enough Juju default to controller-side storage, - otherwise local storage is used. + and local storage otherwise. Recommended usage: diff --git a/ops/_main.py b/ops/_main.py index ff09cbc7e..d0eeab6b8 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -530,7 +530,7 @@ def run(self): def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None): """Set up the charm and dispatch the observed event. - See ``ops.main`` for details. + See `ops.main() <#ops-main-entry-point>`_ for details. """ try: manager = _Manager(charm_class, use_juju_for_storage=use_juju_for_storage) diff --git a/ops/main.py b/ops/main.py index 032288bb6..a4ccfdbe1 100644 --- a/ops/main.py +++ b/ops/main.py @@ -27,7 +27,7 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[ .. deprecated:: 2.16.0 This entrypoint has been deprecated, use `ops.main()` instead. - See ``ops.main()`` for details. + See `ops.main() <#ops-main-entry-point>`_ for details. """ warnings.warn( 'Calling `ops.main.main()` is deprecated, call `ops.main()` instead', diff --git a/test/test_main_invocation.py b/test/test_main_invocation.py index 40f9c97bd..70efd2745 100644 --- a/test/test_main_invocation.py +++ b/test/test_main_invocation.py @@ -39,15 +39,10 @@ def charm_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): os.environ.pop('OPERATOR_DISPATCH', None) -class IdleCharm(ops.CharmBase): - def __init__(self, framework: ops.Framework): - super().__init__(framework) - - def test_top_level_import(charm_env: None): import ops - ops.main(IdleCharm) + ops.main(ops.CharmBase) with pytest.raises(TypeError): ops.main() # type: ignore @@ -57,7 +52,7 @@ def test_top_level_import_legacy_call(charm_env: None): import ops with pytest.deprecated_call(): - ops.main.main(IdleCharm) + ops.main.main(ops.CharmBase) with pytest.raises(TypeError): ops.main.main() # type: ignore @@ -66,7 +61,7 @@ def test_top_level_import_legacy_call(charm_env: None): def test_submodule_import(charm_env: None): import ops.main - ops.main(IdleCharm) # type: ignore # https://github.com/microsoft/pyright/issues/8830 + ops.main(ops.CharmBase) # type: ignore # https://github.com/microsoft/pyright/issues/8830 with pytest.raises(TypeError): ops.main() # type: ignore @@ -76,7 +71,7 @@ def test_submodule_import_legacy_call(charm_env: None): import ops.main with pytest.deprecated_call(): - ops.main.main(IdleCharm) + ops.main.main(ops.CharmBase) with pytest.raises(TypeError): ops.main.main() # type: ignore @@ -85,7 +80,7 @@ def test_submodule_import_legacy_call(charm_env: None): def test_import_from_top_level_module(charm_env: None): from ops import main - main(IdleCharm) + main(ops.CharmBase) with pytest.raises(TypeError): main() # type: ignore @@ -95,7 +90,7 @@ def test_import_from_top_level_module_legacy_call(charm_env: None): from ops import main with pytest.deprecated_call(): - main.main(IdleCharm) + main.main(ops.CharmBase) with pytest.raises(TypeError): main.main() # type: ignore @@ -105,7 +100,7 @@ def test_legacy_import_from_submodule(charm_env: None): from ops.main import main with pytest.deprecated_call(): - main(IdleCharm) + main(ops.CharmBase) with pytest.raises(TypeError): main() # type: ignore diff --git a/test/test_main_type_hint.py b/test/test_main_type_hint.py index 7617eb695..96fd8d835 100644 --- a/test/test_main_type_hint.py +++ b/test/test_main_type_hint.py @@ -44,30 +44,30 @@ def type_test_negative(_arg: Callable[[], None]): def top_level_import(): import ops - type_test_dummy(ops.main.__call__) # pyright is quirky + type_test_dummy(ops.main) type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main) # type: ignore type_test_negative(ops.main.main) # type: ignore def submodule_import(): import ops.main - type_test_dummy(ops.main.__call__) # type: ignore # https://github.com/microsoft/pyright/issues/8830 + type_test_dummy(ops.main) # type: ignore # https://github.com/microsoft/pyright/issues/8830 type_test_dummy(ops.main.main) - type_test_negative(ops.main.__call__) # type: ignore + type_test_negative(ops.main) # type: ignore type_test_negative(ops.main.main) # type: ignore def import_from_top_level_module(): from ops import main - type_test_dummy(main.__call__) + type_test_dummy(main) type_test_dummy(main.main) - type_test_negative(main.__call__) # type: ignore + type_test_negative(main) # type: ignore type_test_negative(main.main) # type: ignore From 8de35e39317a993934b520bcdd7fe90bf58cceff Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 14:04:56 +0900 Subject: [PATCH 27/33] address review comments --- ops/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ops/__init__.py b/ops/__init__.py index 05d153dcc..0e484d5df 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -363,5 +363,6 @@ def main( class SomeCharm(ops.CharmBase): ... - ops.main(SomeCharm) + if __name__ == "__main__": + ops.main(SomeCharm) """ From b0379f7b719fbc13045147c84f6f2015b880c3e8 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 29 Aug 2024 15:04:32 +0900 Subject: [PATCH 28/33] move argv[0] where it belongs --- ops/__init__.py | 25 ++++++++++++------------- ops/_main.py | 9 +++++++-- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index 0e484d5df..2a2c8f47a 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -340,19 +340,6 @@ def main( main = _Main() """Set up the charm and dispatch the observed event. -The event name is based on the way this executable was called (argv[0]). - -.. jujuremoved:: 4.0 - The ``use_juju_for_storage`` argument is not available from Juju 4.0 - -Args: - charm_class: the charm class to instantiate and receive the event. - use_juju_for_storage: whether to use controller-side storage. - The default is ``False`` for most charms. - Podspec charms that haven't previously used local storage and that - are running on a new enough Juju default to controller-side storage, - and local storage otherwise. - Recommended usage: .. code-block:: python @@ -363,6 +350,18 @@ def main( class SomeCharm(ops.CharmBase): ... + if __name__ == "__main__": ops.main(SomeCharm) + +Args: + charm_class: the charm class to instantiate and receive the event. + use_juju_for_storage: whether to use controller-side storage. + The default is ``False`` for most charms. + Podspec charms that haven't previously used local storage and that + are running on a new enough Juju default to controller-side storage, + and local storage otherwise. + +.. jujuremoved:: 4.0 + The ``use_juju_for_storage`` argument is not available from Juju 4.0 """ diff --git a/ops/_main.py b/ops/_main.py index d0eeab6b8..a49284652 100644 --- a/ops/_main.py +++ b/ops/_main.py @@ -216,8 +216,13 @@ def _get_event_args( class _Dispatcher: """Encapsulate how to figure out what event Juju wants us to run. - Also knows how to run “legacy” hooks when Juju called us via a top-level - ``dispatch`` binary. + Juju 2.7.0 and later provide the JUJU_DISPATCH_PATH environment variable. + Earlier versions called individual hook scripts, and that are supported via + two separate mechanisms: + - Charmcraft 0.1.2 and produce `dispatch` shell script that fills this + environment variable if it's missing + - Ops 0.8.0 and later likewise take use ``sys.argv[0]`` if the environment + variable is missing Args: charm_dir: the toplevel directory of the charm From bcffd4306ac1bf7fc6c1186cd5959beb56b6cda4 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 2 Sep 2024 12:17:32 +0900 Subject: [PATCH 29/33] update type tests to mirror backwards compatiblity --- test/test_main_type_hint.py | 101 +++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 36 deletions(-) diff --git a/test/test_main_type_hint.py b/test/test_main_type_hint.py index 96fd8d835..a4fe78319 100644 --- a/test/test_main_type_hint.py +++ b/test/test_main_type_hint.py @@ -11,69 +11,98 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Callable, Type +"""Validate the type signatures on ops.main(). + +This file doesn't contain any run-time tests, rather we rely on pyright to run over this code. +Assignment to a variable declared to follow a protocol is equivalent to backwards compatibility. +""" + +from dataclasses import dataclass +from typing import Optional, Protocol, Type import ops -def type_test_dummy(_arg: Callable[[Type[ops.CharmBase], bool], None]): +class CallableWithCharmClassOnly(Protocol): + """Encapsulate main function type for simple charms. + + Supports: + - ops.main(SomeCharm) + - ops.main(charm_class=SomeCharm) """ - Helper to verify the function signature of ops.main and ops.main.main - Usage: - from somewhere import main + def __call__(self, charm_class: Type[ops.charm.CharmBase]): ... - type_test_dummy(main) - """ +class CallableWithCharmClassAndStorageFlag(Protocol): + """Encapsulate main function type for advanced charms. -def type_test_negative(_arg: Callable[[], None]): + Supports permutations of: + - ops.main(SomeCharm, False) + - ops.main(charm_class=SomeCharm, use_juju_for_storage=False) """ - Helper for negative tests of the function signatures of ops.main and ops.main.main - Usage: - from somewhere import main + def __call__( + self, charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None + ): ... - type_test_negative(main) # type: ignore - The `reportUnnecessaryTypeIgnoreComment` setting is expected to kick up a fuss, - should the passed argument match the expected argument type. +class CallableWithoutArguments(Protocol): + """Bad charm code should be caught by type checker. + + For example: + - ops.main() # type: ignore or pyright complains """ + def __call__(self): ... -def top_level_import(): - import ops - type_test_dummy(ops.main) - type_test_dummy(ops.main.main) +@dataclass +class MainCalls: + simple: CallableWithCharmClassOnly + full: CallableWithCharmClassAndStorageFlag + bad: CallableWithoutArguments - type_test_negative(ops.main) # type: ignore - type_test_negative(ops.main.main) # type: ignore +sink = MainCalls(None, None, None) # type: ignore -def submodule_import(): - import ops.main - type_test_dummy(ops.main) # type: ignore # https://github.com/microsoft/pyright/issues/8830 - type_test_dummy(ops.main.main) +def top_level_import() -> None: + import ops - type_test_negative(ops.main) # type: ignore - type_test_negative(ops.main.main) # type: ignore + sink.full = ops.main + sink.full = ops.main.main + sink.simple = ops.main.main + sink.simple = ops.main.main + sink.bad = ops.main # type: ignore[assignment] + sink.bad = ops.main.main # type: ignore[assignment] -def import_from_top_level_module(): - from ops import main +def submodule_import() -> None: + import ops.main - type_test_dummy(main) - type_test_dummy(main.main) + sink.full = ops.main # type: ignore # type checker limitation https://github.com/microsoft/pyright/issues/8830 + sink.full = ops.main.main + sink.simple = ops.main # type: ignore # https://github.com/microsoft/pyright/issues/8830 + sink.simple = ops.main.main + sink.bad = ops.main # type: ignore[assignment] + sink.bad = ops.main.main # type: ignore[assignment] - type_test_negative(main) # type: ignore - type_test_negative(main.main) # type: ignore +def import_from_top_level_module() -> None: + from ops import main + + sink.full = main + sink.full = main.main + sink.simple = main + sink.simple = main.main + sink.bad = main # type: ignore[assignment] + sink.bad = main.main # type: ignore[assignment] -def import_from_submodule(): - from ops.main import main - type_test_dummy(main) +def import_from_submodule() -> None: + from ops.main import main - type_test_negative(main) # type: ignore + sink.full = main + sink.simple = main + sink.bad = main # type: ignore[assignment] From a2f724ec1cc25e72d9db6fd38c876f157b7a4ebf Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 2 Sep 2024 12:18:44 +0900 Subject: [PATCH 30/33] compact docstring --- ops/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index 2a2c8f47a..19679cfec 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -346,10 +346,7 @@ def main( import ops - - class SomeCharm(ops.CharmBase): - ... - + class SomeCharm(ops.CharmBase): ... if __name__ == "__main__": ops.main(SomeCharm) From 64e2184a54d5df8ca622021ea0dbe252b32886d6 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 2 Sep 2024 12:22:51 +0900 Subject: [PATCH 31/33] copyright header spacing --- test/test_main_invocation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_main_invocation.py b/test/test_main_invocation.py index 70efd2745..4751b7fd1 100644 --- a/test/test_main_invocation.py +++ b/test/test_main_invocation.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import os from pathlib import Path from unittest.mock import Mock From ede45079ec2999d414fdf4052e200bf31b743039 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 2 Sep 2024 14:06:59 +0900 Subject: [PATCH 32/33] re-export symbols that scenario 6 uses --- ops/main.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ops/main.py b/ops/main.py index a4ccfdbe1..2ae1aebd1 100644 --- a/ops/main.py +++ b/ops/main.py @@ -20,6 +20,14 @@ from . import _main +# Re-export specific set of symbols that Scenario 6 imports from ops.main +from ._main import ( # noqa: F401 + CHARM_STATE_FILE, # type: ignore[reportUnusedImport] + _Dispatcher, # type: ignore[reportUnusedImport] + _get_event_args, # type: ignore[reportUnusedImport] + logger, # type: ignore[reportUnusedImport] +) + def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[bool] = None): """Legacy entrypoint to set up the charm and dispatch the observed event. From 8c0290d3800934f9c4841a2852c113f93e836595 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Mon, 2 Sep 2024 15:38:28 +0900 Subject: [PATCH 33/33] test bug --- test/test_main_type_hint.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/test_main_type_hint.py b/test/test_main_type_hint.py index a4fe78319..25bfde9ce 100644 --- a/test/test_main_type_hint.py +++ b/test/test_main_type_hint.py @@ -70,10 +70,10 @@ class MainCalls: def top_level_import() -> None: import ops + sink.simple = ops.main + sink.simple = ops.main.main sink.full = ops.main sink.full = ops.main.main - sink.simple = ops.main.main - sink.simple = ops.main.main sink.bad = ops.main # type: ignore[assignment] sink.bad = ops.main.main # type: ignore[assignment] @@ -81,10 +81,10 @@ def top_level_import() -> None: def submodule_import() -> None: import ops.main + sink.simple = ops.main # type: ignore # type checker limitation https://github.com/microsoft/pyright/issues/8830 + sink.simple = ops.main.main sink.full = ops.main # type: ignore # type checker limitation https://github.com/microsoft/pyright/issues/8830 sink.full = ops.main.main - sink.simple = ops.main # type: ignore # https://github.com/microsoft/pyright/issues/8830 - sink.simple = ops.main.main sink.bad = ops.main # type: ignore[assignment] sink.bad = ops.main.main # type: ignore[assignment] @@ -92,10 +92,10 @@ def submodule_import() -> None: def import_from_top_level_module() -> None: from ops import main - sink.full = main - sink.full = main.main sink.simple = main sink.simple = main.main + sink.full = main + sink.full = main.main sink.bad = main # type: ignore[assignment] sink.bad = main.main # type: ignore[assignment] @@ -103,6 +103,6 @@ def import_from_top_level_module() -> None: def import_from_submodule() -> None: from ops.main import main - sink.full = main sink.simple = main + sink.full = main sink.bad = main # type: ignore[assignment]