Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should actions leak deprecation warnings? #1460

Open
PietroPasotti opened this issue Nov 20, 2024 · 2 comments
Open

should actions leak deprecation warnings? #1460

PietroPasotti opened this issue Nov 20, 2024 · 2 comments
Labels
25.04 bug Something isn't working small item

Comments

@PietroPasotti
Copy link
Contributor

In my view, juju actions are the public facade of the charm. The user-facing API.
A user doesn't necessarily want to see things such as internal deprecation warnings or other logs as part of the action output, or they might think something went wrong when running the action.

I'm not sure if this is a juju or an ops concern, but this user experience should be improved:

image

What do you think is the way forward on this? Silence all stdout when running an action? This warning will show up in the juju debug-log anyway, which is the only place I'd expect to see it.

@benhoyt
Copy link
Collaborator

benhoyt commented Nov 20, 2024

Yeah, I agree these shouldn't end up in the actions output. Hopefully there's a straight-forward way to make these warnings only go to the debug log. We'll aim to look into this at some point soon.

@tonyandrewmeyer
Copy link
Contributor

tonyandrewmeyer commented Nov 24, 2024

Odd, these shouldn't end up being output, because the default filter ignores DeprecationWarning except for code in __main__.

I tested this with this little charm:

import ops
import ops.main

class ActionwarningCharm(ops.CharmBase):
    def __init__(self, framework: ops.Framework):
        super().__init__(framework)
        framework.observe(self.on.go_action, self._on_go)

    def _on_go(self, event):
        event.log("Running...")
        event.set_results({"result": "done"})

if __name__ == "__main__":
    ops.main.main(ActionwarningCharm)

And when running the action, I don't get the warning. It is being produced: if I put PYTHONWARNINGS="error::DeprecationWarning" into the dispatch script then I correctly get an error.

@PietroPasotti would any of the code be setting PYTHONWARNINGS or running the script with a -W argument or calling warnings.filterwarnings?

It seems like the correct fix is to to set the filters appropriately, but that's not going to help if something is changing them. If that isn't going to work, then we might have to use a custom showwarning that redirects to logging/debug-log rather than relying on stderr output (as we investigated and rejected previously).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
25.04 bug Something isn't working small item
Projects
None yet
Development

No branches or pull requests

3 participants