Skip to content

Commit

Permalink
Merge pull request #1580 from mathbunnyru/asalikhov/fix_typing
Browse files Browse the repository at this point in the history
Fix all typing issues
  • Loading branch information
mathbunnyru committed Jan 24, 2022
2 parents 9d13d95 + 37c510f commit b020a0c
Show file tree
Hide file tree
Showing 25 changed files with 184 additions and 129 deletions.
7 changes: 7 additions & 0 deletions .pre-commit-config.yaml
Expand Up @@ -17,6 +17,13 @@ repos:
- id: black
args: [--target-version=py39]

# Check python code static typing
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.931
hooks:
- id: mypy
additional_dependencies: ["pytest", "types-requests", "types-tabulate"]

# Autoformat: YAML, JSON, Markdown, etc.
- repo: https://github.com/pre-commit/mirrors-prettier
rev: v2.5.1
Expand Down
2 changes: 1 addition & 1 deletion all-spark-notebook/test/test_spark_notebooks.py
Expand Up @@ -3,7 +3,7 @@

import logging

import pytest
import pytest # type: ignore
from pathlib import Path

from conftest import TrackedContainer
Expand Down
25 changes: 9 additions & 16 deletions base-notebook/jupyter_notebook_config.py
@@ -1,12 +1,12 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

# mypy: ignore-errors
from jupyter_core.paths import jupyter_data_dir
import subprocess
import os
import errno
import stat


c = get_config() # noqa: F821
c.NotebookApp.ip = "0.0.0.0"
c.NotebookApp.port = 8888
Expand All @@ -16,28 +16,21 @@
c.FileContentsManager.delete_to_trash = False

# Generate a self-signed certificate
OPENSSL_CONFIG = """\
[req]
distinguished_name = req_distinguished_name
[req_distinguished_name]
"""
if "GEN_CERT" in os.environ:
dir_name = jupyter_data_dir()
pem_file = os.path.join(dir_name, "notebook.pem")
try:
os.makedirs(dir_name)
except OSError as exc: # Python >2.5
if exc.errno == errno.EEXIST and os.path.isdir(dir_name):
pass
else:
raise
os.makedirs(dir_name, exist_ok=True)

# Generate an openssl.cnf file to set the distinguished name
cnf_file = os.path.join(os.getenv("CONDA_DIR", "/usr/lib"), "ssl", "openssl.cnf")
if not os.path.isfile(cnf_file):
with open(cnf_file, "w") as fh:
fh.write(
"""\
[req]
distinguished_name = req_distinguished_name
[req_distinguished_name]
"""
)
fh.write(OPENSSL_CONFIG)

# Generate a certificate if one doesn't exist on disk
subprocess.check_call(
Expand Down
4 changes: 2 additions & 2 deletions base-notebook/test/test_container_options.py
Expand Up @@ -4,7 +4,7 @@
import time
import logging

import pytest
import pytest # type: ignore
import requests

from conftest import TrackedContainer
Expand Down Expand Up @@ -303,6 +303,6 @@ def test_jupyter_env_vars_to_unset_as_root(
"-c",
"echo I like $FRUIT and ${SECRET_FRUIT:-stuff}, and love ${SECRET_ANIMAL:-to keep secrets}!",
],
**root_args,
**root_args, # type: ignore
)
assert "I like bananas and stuff, and love to keep secrets!" in logs
2 changes: 1 addition & 1 deletion base-notebook/test/test_package_managers.py
Expand Up @@ -2,7 +2,7 @@
# Distributed under the terms of the Modified BSD License.

import logging
import pytest
import pytest # type: ignore

from conftest import TrackedContainer

Expand Down
2 changes: 1 addition & 1 deletion base-notebook/test/test_python.py
Expand Up @@ -2,7 +2,7 @@
# Distributed under the terms of the Modified BSD License.
import logging

from packaging import version
from packaging import version # type: ignore

from conftest import TrackedContainer

Expand Down
2 changes: 1 addition & 1 deletion base-notebook/test/test_start_container.py
Expand Up @@ -3,7 +3,7 @@

import logging
from typing import Optional
import pytest
import pytest # type: ignore
import requests
import time

Expand Down
25 changes: 13 additions & 12 deletions conftest.py
Expand Up @@ -2,11 +2,11 @@
# Distributed under the terms of the Modified BSD License.
import os
import logging
import typing
from typing import Any, Optional

import docker
from docker.models.containers import Container
import pytest
import pytest # type: ignore
import requests

from requests.packages.urllib3.util.retry import Retry
Expand Down Expand Up @@ -35,7 +35,7 @@ def docker_client() -> docker.DockerClient:
@pytest.fixture(scope="session")
def image_name() -> str:
"""Image name to test"""
return os.getenv("TEST_IMAGE")
return os.environ["TEST_IMAGE"]


class TrackedContainer:
Expand All @@ -56,14 +56,14 @@ def __init__(
self,
docker_client: docker.DockerClient,
image_name: str,
**kwargs: typing.Any,
**kwargs: Any,
):
self.container = None
self.docker_client = docker_client
self.image_name = image_name
self.kwargs = kwargs
self.container: Optional[Container] = None
self.docker_client: docker.DockerClient = docker_client
self.image_name: str = image_name
self.kwargs: Any = kwargs

def run_detached(self, **kwargs: typing.Any) -> Container:
def run_detached(self, **kwargs: Any) -> Container:
"""Runs a docker container using the preconfigured image name
and a mix of the preconfigured container options and those passed
to this method.
Expand Down Expand Up @@ -94,11 +94,12 @@ def run_and_wait(
timeout: int,
no_warnings: bool = True,
no_errors: bool = True,
**kwargs: typing.Any,
**kwargs: Any,
) -> str:
running_container = self.run_detached(**kwargs)
rv = running_container.wait(timeout=timeout)
logs = running_container.logs().decode("utf-8")
assert isinstance(logs, str)
LOGGER.debug(logs)
if no_warnings:
assert not self.get_warnings(logs)
Expand All @@ -119,14 +120,14 @@ def get_warnings(logs: str) -> list[str]:
def _lines_starting_with(logs: str, pattern: str) -> list[str]:
return [line for line in logs.splitlines() if line.startswith(pattern)]

def remove(self):
def remove(self) -> None:
"""Kills and removes the tracked docker container."""
if self.container:
self.container.remove(force=True)


@pytest.fixture(scope="function")
def container(docker_client: docker.DockerClient, image_name: str):
def container(docker_client: docker.DockerClient, image_name: str) -> Container:
"""Notebook container with initial configuration appropriate for testing
(e.g., HTTP port exposed to the host for HTTP calls).
Expand Down
2 changes: 1 addition & 1 deletion minimal-notebook/test/test_nbconvert.py
Expand Up @@ -3,7 +3,7 @@

import logging

import pytest
import pytest # type: ignore
from pathlib import Path

from conftest import TrackedContainer
Expand Down
26 changes: 26 additions & 0 deletions mypy.ini
@@ -0,0 +1,26 @@
[mypy]
python_version = 3.9
follow_imports = normal
strict = False
no_incremental = True

[mypy-docker.*]
ignore_missing_imports = True

[mypy-matplotlib.*]
ignore_missing_imports = True

[mypy-packaging.*]
ignore_missing_imports = True

[mypy-pandas.*]
ignore_missing_imports = True

[mypy-plumbum.*]
ignore_missing_imports = True

[mypy-pyspark.*]
ignore_missing_imports = True

[mypy-tensorflow.*]
ignore_missing_imports = True
1 change: 1 addition & 0 deletions pyspark-notebook/ipython_kernel_config.py
Expand Up @@ -10,4 +10,5 @@
# Attempt to capture and forward low-level output, e.g. produced by Extension
# libraries.
# Default: True
# type:ignore
c.IPKernelApp.capture_fd_output = False # noqa: F821
2 changes: 1 addition & 1 deletion scipy-notebook/test/test_extensions.py
Expand Up @@ -2,7 +2,7 @@
# Distributed under the terms of the Modified BSD License.
import logging

import pytest
import pytest # type: ignore

from conftest import TrackedContainer

Expand Down
4 changes: 2 additions & 2 deletions scipy-notebook/test/test_matplotlib.py
Expand Up @@ -3,7 +3,7 @@

import logging

import pytest
import pytest # type: ignore
from pathlib import Path

from conftest import TrackedContainer
Expand All @@ -29,7 +29,7 @@
)
def test_matplotlib(
container: TrackedContainer, test_file: str, expected_file: str, description: str
):
) -> None:
"""Various tests performed on matplotlib
- Test that matplotlib is able to plot a graph and write it as an image
Expand Down
5 changes: 3 additions & 2 deletions tagging/create_manifests.py
Expand Up @@ -5,6 +5,7 @@
import datetime
import logging
import os
from docker.models.containers import Container
from .docker_runner import DockerRunner
from .get_taggers_and_manifests import get_taggers_and_manifests
from .git_helper import GitHelper
Expand Down Expand Up @@ -55,9 +56,9 @@ def create_manifest_file(
owner: str,
wiki_path: str,
manifests: list[ManifestInterface],
container,
container: Container,
) -> None:
manifest_names = [manifest.__name__ for manifest in manifests]
manifest_names = [manifest.__class__.__name__ for manifest in manifests]
LOGGER.info(f"Using manifests: {manifest_names}")

commit_hash_tag = GitHelper.commit_hash_tag()
Expand Down
12 changes: 10 additions & 2 deletions tagging/docker_runner.py
@@ -1,6 +1,7 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.
from typing import Optional
from types import TracebackType
import docker
from docker.models.containers import Container
import logging
Expand All @@ -13,7 +14,7 @@ class DockerRunner:
def __init__(
self,
image_name: str,
docker_client=docker.from_env(),
docker_client: docker.DockerClient = docker.from_env(),
command: str = "sleep infinity",
):
self.container: Optional[Container] = None
Expand All @@ -31,7 +32,13 @@ def __enter__(self) -> Container:
LOGGER.info(f"Container {self.container.name} created")
return self.container

def __exit__(self, exc_type, exc_value, traceback) -> None:
def __exit__(
self,
exc_type: Optional[type[BaseException]],
exc_val: Optional[BaseException],
exc_tb: Optional[TracebackType],
) -> None:
assert self.container is not None
LOGGER.info(f"Removing container {self.container.name} ...")
if self.container:
self.container.remove(force=True)
Expand All @@ -44,6 +51,7 @@ def run_simple_command(
LOGGER.info(f"Running cmd: '{cmd}' on container: {container}")
out = container.exec_run(cmd)
result = out.output.decode("utf-8").rstrip()
assert isinstance(result, str)
if print_result:
LOGGER.info(f"Command result: {result}")
assert out.exit_code == 0, f"Command: {cmd} failed"
Expand Down
22 changes: 12 additions & 10 deletions tagging/get_taggers_and_manifests.py
@@ -1,20 +1,22 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.
from typing import Optional
from .images_hierarchy import ALL_IMAGES
from .manifests import ManifestInterface
from .taggers import TaggerInterface


def get_taggers_and_manifests(
short_image_name: str,
short_image_name: Optional[str],
) -> tuple[list[TaggerInterface], list[ManifestInterface]]:
taggers: list[TaggerInterface] = []
manifests: list[ManifestInterface] = []
while short_image_name is not None:
image_description = ALL_IMAGES[short_image_name]
if short_image_name is None:
return [[], []] # type: ignore

taggers = image_description.taggers + taggers
manifests = image_description.manifests + manifests

short_image_name = image_description.parent_image
return taggers, manifests
image_description = ALL_IMAGES[short_image_name]
parent_taggers, parent_manifests = get_taggers_and_manifests(
image_description.parent_image
)
return (
parent_taggers + image_description.taggers,
parent_manifests + image_description.manifests,
)
4 changes: 2 additions & 2 deletions tagging/git_helper.py
Expand Up @@ -7,15 +7,15 @@
class GitHelper:
@staticmethod
def commit_hash() -> str:
return git["rev-parse", "HEAD"]().strip()
return git["rev-parse", "HEAD"]().strip() # type: ignore

@staticmethod
def commit_hash_tag() -> str:
return GitHelper.commit_hash()[:12]

@staticmethod
def commit_message() -> str:
return git["log", -1, "--pretty=%B"]().strip()
return git["log", -1, "--pretty=%B"]().strip() # type: ignore


if __name__ == "__main__":
Expand Down
2 changes: 1 addition & 1 deletion tagging/github_set_env.py
Expand Up @@ -3,7 +3,7 @@
import os


def github_set_env(env_name, env_value):
def github_set_env(env_name: str, env_value: str) -> None:
if not os.environ.get("GITHUB_ACTIONS") or not os.environ.get("GITHUB_ENV"):
return

Expand Down

0 comments on commit b020a0c

Please sign in to comment.