Add type annotations #434
Add type annotations #434
Conversation
Codecov Report
@@ Coverage Diff @@
## master #434 +/- ##
==========================================
- Coverage 68.37% 68.06% -0.32%
==========================================
Files 23 23
Lines 623 645 +22
==========================================
+ Hits 426 439 +13
- Misses 197 206 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
.pre-commit-config.yaml
Outdated
| # See https://pre-commit.com/hooks.html for more hooks | ||
| repos: | ||
| - repo: https://github.com/psf/black | ||
| rev: 21.5b1 |
There was a problem hiding this comment.
can this versions be taken from requirements file? Otherwise I might forget to update it manually and I'm not sure if dependabot will update it by itself 🤔
There was a problem hiding this comment.
I could change this to instead use only packages installed in the system, i.e. the ones from our virtualenv. That might be better here, while less consistent with usual usage of pre-commit.
| connection = Any | ||
| if not TYPE_CHECKING: | ||
| # if there's no postgres, just go with the flow. | ||
| # pylint:disable=invalid-name |
There was a problem hiding this comment.
oh... we can rid of pylint comment here
There was a problem hiding this comment.
Are we intentionally not using pylint? Should I (separately) delete all such comments or fix it so pylint passes?
There was a problem hiding this comment.
intentionally for now.
I left the formatting and most of the checks to black. As it often happened that one breaks the other and it takes a bit of tinkering with configuration for them to cooperate.
There was a problem hiding this comment.
If it's ok, I'd prefer to delete the unused ignores in a separate PR.
src/pytest_postgresql/executor.py
Outdated
| @@ -87,7 +90,7 @@ def __init__( | |||
| :param int port: port under which process is accessible | |||
| :param str datadir: path to postgresql datadir | |||
| :param str unixsocketdir: path to socket directory | |||
| :param pathlib.Path logfile: path to logfile for postgresql | |||
| :param str logfile: path to logfile for postgresql | |||
.pre-commit-config.yaml
Outdated
| # See https://pre-commit.com/hooks.html for more hooks | ||
| repos: | ||
| - repo: https://github.com/psf/black | ||
| rev: 21.5b1 |
There was a problem hiding this comment.
I could change this to instead use only packages installed in the system, i.e. the ones from our virtualenv. That might be better here, while less consistent with usual usage of pre-commit.
| """Issue a stop request to executable.""" | ||
| subprocess.check_output( | ||
| f"{self.executable} stop -D {self.datadir} -m f", | ||
| shell=True, | ||
| ) | ||
| try: | ||
| super().stop(sig, exp_sig) | ||
| super().stop(sig, exp_sig) # type: ignore[arg-type] |
There was a problem hiding this comment.
I think this one results from mirakuru not using strict optional.
| @@ -59,28 +60,33 @@ def get_port(ports): | |||
| if ports == -1: | |||
| return None | |||
There was a problem hiding this comment.
Do we need that? I'm not sure if there is any situation where None returned from get_port is not a configuration error. In port_for.select_random lack of available ports causes an exception, so this function is not guaranteed to return None instead of raising an exception.
I think it would be simpler to always return an available port or raise an exception.
There was a problem hiding this comment.
Ok, it's for pg_port = get_port(port) or get_port(config["port"]).
| connection = Any | ||
| if not TYPE_CHECKING: | ||
| # if there's no postgres, just go with the flow. | ||
| # pylint:disable=invalid-name |
There was a problem hiding this comment.
intentionally for now.
I left the formatting and most of the checks to black. As it often happened that one breaks the other and it takes a bit of tinkering with configuration for them to cooperate.
mmaslowskicc
force-pushed
the
type-annotations
branch
from
3c0a629 to
3f846ad
Compare
Fixes #432.
Type annotations, mypy configuration and needed fixes are added.