Skip to content

Commit

Permalink
Go back to issuing a real warning.
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyandrewmeyer committed Dec 11, 2024
1 parent 58b7a59 commit eca45fb
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 31 deletions.
46 changes: 29 additions & 17 deletions ops/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
"""Support legacy ops.main.main() import."""

import inspect
import logging
from typing import Any, Optional, Type
import os
import warnings
from typing import Any, Optional, Type, Union

import ops.charm

Expand All @@ -27,10 +28,9 @@
CHARM_STATE_FILE, # type: ignore[reportUnusedImport]
_Dispatcher, # type: ignore[reportUnusedImport]
_get_event_args, # type: ignore[reportUnusedImport]
logger, # type: ignore[reportUnusedImport]
)

logger = logging.getLogger(__name__)


def _top_frame():
frame = inspect.currentframe()
Expand All @@ -56,13 +56,7 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[
# This means that we need to delay emitting the warning until the framework
# has been set up, so we wrap the charm and do it on instantiation. However,
# this means that a regular warning call won't provide the correct filename
# and line number - we could wrap this in a custom formatter, but it's
# simpler to just use a logging.warning() call instead.
# Note that unit tests almost never call main(), so wouldn't show this
# warning, so we're relying on integration tests, which aren't going to be
# treating warning calls in any special way. That means that we can just
# output the warning directly as a log message, and the effect will be the
# same.
# and line number.
# Note also that this will be logged with every event. Our assumption is
# that this will be noticeable enough during integration testing that it
# will get fixed before going into production.
Expand All @@ -72,12 +66,30 @@ def main(charm_class: Type[ops.charm.CharmBase], use_juju_for_storage: Optional[
class DeprecatedMainCharmBase(charm_class):
def __init__(self, *args: Any, **kwargs: Any):
super().__init__(*args, **kwargs)
logger.warning(
'%s:%s: DeprecationWarning: Calling `ops.main()` is deprecated, '
'call `ops.main()` instead',
frame.f_code.co_filename,
frame.f_lineno,
)

_original_format = warnings.formatwarning

def custom_warning_formatter(
message: Union[str, Warning],
category: Type[Warning],
*args: Any,
**kwargs: Any,
) -> str:
"""Like the default formatter, but patch in the filename and line number."""
return (
f'{frame.f_code.co_filename}:{frame.f_lineno}: '
f'{category.__name__}: {message}'
)

try:
warnings.formatwarning = custom_warning_formatter
warnings.warn(
'Calling `ops.main()` is deprecated, call `ops.main()` instead',
DeprecationWarning,
stacklevel=2,
)
finally:
warnings.formatwarning = _original_format

return _main.main(
charm_class=DeprecatedMainCharmBase, use_juju_for_storage=use_juju_for_storage
Expand Down
26 changes: 12 additions & 14 deletions test/test_main_invocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ def test_top_level_import(charm_env: None):
ops.main() # type: ignore


def test_top_level_import_legacy_call(charm_env: None, caplog: pytest.LogCaptureFixture):
def test_top_level_import_legacy_call(charm_env: None):
import ops

ops.main.main(ops.CharmBase)
assert 'DeprecationWarning: Calling `ops.main()` is deprecated' in caplog.records[0].message
with pytest.deprecated_call():
ops.main.main(ops.CharmBase)

with pytest.raises(TypeError):
ops.main.main() # type: ignore
Expand All @@ -68,11 +68,11 @@ def test_submodule_import(charm_env: None):
ops.main() # type: ignore


def test_submodule_import_legacy_call(charm_env: None, caplog: pytest.LogCaptureFixture):
def test_submodule_import_legacy_call(charm_env: None):
import ops.main

ops.main.main(ops.CharmBase)
assert 'DeprecationWarning: Calling `ops.main()` is deprecated' in caplog.records[0].message
with pytest.deprecated_call():
ops.main.main(ops.CharmBase)

with pytest.raises(TypeError):
ops.main.main() # type: ignore
Expand All @@ -87,23 +87,21 @@ def test_import_from_top_level_module(charm_env: None):
main() # type: ignore


def test_import_from_top_level_module_legacy_call(
charm_env: None, caplog: pytest.LogCaptureFixture
):
def test_import_from_top_level_module_legacy_call(charm_env: None):
from ops import main

main.main(ops.CharmBase)
assert 'DeprecationWarning: Calling `ops.main()` is deprecated' in caplog.records[0].message
with pytest.deprecated_call():
main.main(ops.CharmBase)

with pytest.raises(TypeError):
main.main() # type: ignore


def test_legacy_import_from_submodule(charm_env: None, caplog: pytest.LogCaptureFixture):
def test_legacy_import_from_submodule(charm_env: None):
from ops.main import main

main(ops.CharmBase)
assert 'DeprecationWarning: Calling `ops.main()` is deprecated' in caplog.records[0].message
with pytest.deprecated_call():
main(ops.CharmBase)

with pytest.raises(TypeError):
main() # type: ignore

0 comments on commit eca45fb

Please sign in to comment.