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

fix: tell type checkers that the config options are strings #241

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

tonyandrewmeyer
Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer commented Apr 11, 2024

Issue

ops<=2.12 wrongly has self.config[x] typed as str, when actually it could be an int, float, bool, or str, depending on the config type. We're fixing this in ops:#1183, but that will break static checking that currently assumes that the config is a str (because ops doesn't validate the schema, so all options will be bool|int|float|str).

Solution

Two typing.cast calls where the config values are loaded and used where the type should be str.

Context

Covered above.

Testing Instructions

Run tox -e static-charm with/without the PR with ops 2.12 and with either the PR branch linked above or once that's merged ops main/2.13.

Upgrade Notes

N/A

@tonyandrewmeyer
Copy link
Contributor Author

Let me know if there's anything else I need to do here - if we can get this merged then we can enable the CI tests in ops against this charm, which would be great :). Thanks!

@lucabello lucabello merged commit de66044 into canonical:main Apr 26, 2024
12 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the fix-config-type branch May 24, 2024 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants