Skip to content

Enable mypy strict mode#8766

Merged
bmw merged 45 commits intocertbot:masterfrom
adferrand:mypy-812-strict
Apr 5, 2021
Merged

Enable mypy strict mode#8766
bmw merged 45 commits intocertbot:masterfrom
adferrand:mypy-812-strict

Conversation

@adferrand
Copy link
Copy Markdown
Collaborator

Built on top of #8748, this PR reenables mypy strict mode and adds the appropriate corrections to pass the types checks.

@adferrand adferrand changed the title Enable strict mypy strict mode Enable mypy strict mode Apr 2, 2021
@bmw bmw self-assigned this Apr 2, 2021
Copy link
Copy Markdown
Member

@bmw bmw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than my inline comments, I think we should remove --no-strict-optional from tox.ini.

def __init__(self, entry_point: pkg_resources.EntryPoint, with_prefix=False):
self.name = self.entry_point_to_plugin_name(entry_point, with_prefix)
self.plugin_cls = entry_point.load()
self.plugin_cls: Any = entry_point.load()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I change this to interfaces.IPluginFactory which is what this object should be, it almost works, however, mypy complains about the number of parameters when we call self.plugin_cls(...).

Do you think it's worth either of:

  1. Using that interface as the type here and either fixing it or adding type: ignore lines to make mypy happy?
  2. Adding a comment saying what this type can be in the future when we're off of zope.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option 1. Once we have removed the interface because we moved away from zope, this code will need a fix so we cannot miss the occasion to improve the type definition here.

self._initialized = None
self._prepared = None
self.warning_message: Optional[str] = None
self._initialized: Optional[Any] = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a similar comment here as I do about self.plugin_cls except here the type should be interfaces.IPlugin and mypy complains about the number of parameters on the prepare call.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

Copy link
Copy Markdown
Member

@bmw bmw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@bmw bmw merged commit c438a39 into certbot:master Apr 5, 2021
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