Update Pyright version to latest (1.1.313) #944
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Boy this was more painful than I expected. Lots of fighting with the compiler. I ended up disabling a few more Pyright issues, specifically these seemed more trouble than they're worth: * reportPrivateUsage: we do this often in the codebase, for example charm.py pokes at _ModelBackend stuff * reportUnnecessaryIsInstance: we do lots of isinstance checks to detect type issues at runtime (we've done this since the beginning, and it's useful for people not using type checking) * reportUnnecessaryComparison: similar to the above, but for checking "if non_optional_value is None" and the like
benhoyt
changed the title
|
as for the failing o11y static checks, yeah, I know strings, bytes, buffers and the like can be a humongous pain. I've wrestled with those just as hard last year. Have no specific suggestions on how to address it here. Curious to see if the error disappears if we use the previous pyright version? Or is it a consequence of the changes you made? |
Currently Pyright isn't (usually?) able to find the return type of
Container.exec, I think due to an import ordering thing? As such,
charms that use it get an Any type and static checks pass.
However, as soon as Pyright *can* find the return type, it's not really
correct, and we get errors like shown below.
We need to use Generic[AnyStr] and various overloads to ensure the
caller gets an ExecProcess[str] if they call exec() with "encoding" set
(the default), or ExecProcess[bytes] if they call exec() with
"encoding" set to None.
$ tox -e static-charm
static-charm: commands[0]> pyright /home/ben/w/grafana-k8s-operator/src
/home/ben/w/grafana-k8s-operator/src/charm.py
/home/ben/w/grafana-k8s-operator/src/charm.py:929:28 - error: Argument of type "Literal['Version (\\d*\\.\\d*\\.\\d*)']" cannot be assigned to parameter "pattern" of type "bytes | Pattern[bytes]" in function "search"
Type "Literal['Version (\\d*\\.\\d*\\.\\d*)']" cannot be assigned to type "bytes | Pattern[bytes]"
"Literal['Version (\\d*\\.\\d*\\.\\d*)']" is incompatible with "bytes"
"Literal['Version (\\d*\\.\\d*\\.\\d*)']" is incompatible with "Pattern[bytes]" (reportGeneralTypeIssues)
/home/ben/w/grafana-k8s-operator/src/charm.py:929:56 - error: Argument of type "_StrOrBytes" cannot be assigned to parameter "string" of type "ReadableBuffer" in function "search"
Type "_StrOrBytes" cannot be assigned to type "ReadableBuffer"
Type "str" cannot be assigned to type "ReadableBuffer"
"str" is incompatible with "ReadOnlyBuffer"
"str" is incompatible with "bytearray"
"str" is incompatible with "memoryview"
"str" is incompatible with "array[Any]"
"str" is incompatible with "mmap"
"str" is incompatible with "_CData"
... (reportGeneralTypeIssues)
2 errors, 0 warnings, 0 informations
|
Created https://github.com/canonical/alertmanager-k8s-operator-1/pull/1 to fix the Pyright issue with CI there. |
jnsgruk
approved these changes
Jun 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates Pyright to the latest version, 1.1.313, and fixes the errors that resulted.
Boy this was more painful than I expected. It seems to have gotten significantly stricter. I engaged in lots of fighting with the compiler -- if I knew any swear words I'd be using them here. In one case (
_ModelBackend._run) I stopped fighting and added atype: ignore. I think we need to be pragmatic here: the code is no worse than it was before, and we can always improve specific things in future PRs.I also ended up disabling a few more Pyright issues, specifically the following seemed more trouble than they're worth:
_ModelBackend, which is privatePart of #920