Skip to content

Commit

Permalink
pyfmt: enforce isort in CI too
Browse files Browse the repository at this point in the history
This will prevent the next person who runs `bin/pyfmt` from seeing
spurious diffs on their branch.

Also run flake8 to prevent unused imports and variables.
  • Loading branch information
benesch committed Aug 14, 2021
1 parent e88ad32 commit cdbcf54
Show file tree
Hide file tree
Showing 36 changed files with 91 additions and 127 deletions.
2 changes: 1 addition & 1 deletion bin/lint
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fi

if [[ ! "${MZDEV_NO_PYTHON:-}" ]]; then
try bin/pycheck
try bin/pyfmt --diff --check --quiet
try bin/pyfmt --check --diff
if try_last_failed; then
echo "lint: $(red error:) python formatting discrepancies found"
echo "hint: run bin/pyfmt" >&2
Expand Down
19 changes: 3 additions & 16 deletions bin/pycheck
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,9 @@ cd "$(dirname "$0")/.."

. misc/shlib/shlib.bash

py_files=$(git_files "ci/*.py" "misc/python/*.py" "misc/mzutil/scripts/*.py" "misc/kafka-util/scripts/*.py")
py_folders=(ci misc/kafka-util/scripts misc/mzutil/scripts misc/python)

# Bail out with a nice error message if we discover any syntax errors, so that
# mypy and pytest don't spew nonsense.
if ! bin/pyactivate --dev -m compileall -q -l -i - <<< "$py_files"; then
echo "pycheck: $(red error:) python syntax errors found"
exit 1
fi

try xargs bin/pyactivate --dev -m mypy \
--pretty \
--no-error-summary \
--namespace-packages \
--explicit-package-bases \
--python-version 3.8 \
<<< "$py_files"
try bin/pyactivate --dev -m mypy "${py_folders[@]}"
try bin/pyactivate --dev -m flake8 --select F --ignore F541 --extend-exclude venv "${py_folders[@]}"
try bin/pyactivate --dev -m pytest -qq --doctest-modules misc/python

try_finish
9 changes: 7 additions & 2 deletions bin/pyfmt
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,10 @@

set -euo pipefail

"$(dirname "$0")"/pyactivate --dev -m black --target-version py38 . "$@"
"$(dirname "$0")"/pyactivate --dev -m isort --profile black .
cd "$(dirname "$0")/.."

. misc/shlib/shlib.bash

try bin/pyactivate --dev -m black . "$@"
try bin/pyactivate --dev -m isort . "$@"
try_finish
2 changes: 1 addition & 1 deletion ci/builder/nightly.stamp
Original file line number Diff line number Diff line change
@@ -1 +1 @@
nightly-20210810-190658
nightly-20210814-011646
2 changes: 1 addition & 1 deletion ci/builder/stable.stamp
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.54.0-20210810-184007
1.54.0-20210814-010705
1 change: 0 additions & 1 deletion ci/cleanup/aws.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from urllib.parse import unquote, urlparse

import boto3
import botocore

from materialize import scratch

Expand Down
2 changes: 1 addition & 1 deletion ci/deploy/deploy_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import boto3
import humanize

from materialize import git, spawn
from materialize import git

APT_BUCKET = "materialize-apt"
BINARIES_BUCKET = "materialize-binaries"
Expand Down
2 changes: 0 additions & 2 deletions ci/deploy/linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
# the Business Source License, use of this software will be governed
# by the Apache License, Version 2.0.

import io
import os
import subprocess
from pathlib import Path

import boto3
Expand Down
3 changes: 1 addition & 2 deletions ci/load/periodic.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@

import datetime

from materialize import scratch
from materialize.cli.scratch import (
DEFAULT_INSTPROF_NAME,
DEFAULT_SG_ID,
DEFAULT_SUBNET_ID,
)

from materialize import scratch


def main() -> None:
desc = scratch.MachineDesc(
Expand Down
3 changes: 2 additions & 1 deletion ci/nightly/coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@
import sys
from pathlib import Path

import materialize.cli.mzcompose
import yaml

import materialize.cli.mzcompose


def main() -> int:
with open(Path(__file__).parent.parent / "test" / "pipeline.template.yml") as f:
Expand Down
1 change: 0 additions & 1 deletion ci/test/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from pathlib import Path

import boto3
import humanize

from materialize import cargo, ci_util, deb, errors, git, mzbuild, spawn

Expand Down
4 changes: 2 additions & 2 deletions ci/test/mkpipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
from collections import OrderedDict
from pathlib import Path
from tempfile import TemporaryFile
from typing import Any, Iterable, List, Sequence, Set
from typing import Any, Iterable, Set

import yaml

from materialize import git, mzbuild, mzcompose, spawn
from materialize import mzbuild, mzcompose, spawn

# These paths contain "CI glue code", i.e., the code that powers CI itself,
# including this very script! All of CI implicitly depends on this code, so
Expand Down
3 changes: 0 additions & 3 deletions misc/mzutil/scripts/wait_for_view_states.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
"""

import argparse
import glob
import io
import json
import os
import pathlib
import sys
Expand Down Expand Up @@ -125,7 +123,6 @@ def wait_for_materialize_views(args: argparse.Namespace) -> None:
if view not in view_snapshots:
continue

sources: typing.List[str] = query_info["sources"]
if len(query_info["sources"]) != 1:
print(
f"ERROR: Expected just one source for view {view}: {query_info['sources']}"
Expand Down
1 change: 0 additions & 1 deletion misc/python/materialize/cli/deploy_antithesis.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
# the Business Source License, use of this software will be governed
# by the Apache License, Version 2.0.

import itertools
import os
from pathlib import Path

Expand Down
8 changes: 3 additions & 5 deletions misc/python/materialize/cli/mkrelease.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@
# the Business Source License, use of this software will be governed
# by the Apache License, Version 2.0.

import base64
import concurrent.futures
import os
import re
import subprocess
import sys
from datetime import date, timedelta
from datetime import date
from pathlib import Path
from typing import Any, Dict, Optional
from typing import Any, Optional

import click
import requests
Expand Down Expand Up @@ -198,7 +197,6 @@ def release(
raise errors.MzConfigurationError(
"working directory is not clean, stash or commit your changes"
)
sys.exit(1)

the_tag = f"v{version}"
confirm_version_is_next(version, affect_remote)
Expand Down Expand Up @@ -364,7 +362,7 @@ def update_upgrade_tests_inner(released_version: Version) -> None:
spawn.capture(
"bin/mzcompose --mz-find upgrade list-workflows".split(), stderr_too=True
)
except subprocess.CalledProcessError as e:
except subprocess.CalledProcessError:
say(
f"The generated test/upgrade/mzcompose.yml file appears to be broken, "
f"please consult {readme}"
Expand Down
1 change: 0 additions & 1 deletion misc/python/materialize/cli/mock_telemetry_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
from http.server import BaseHTTPRequestHandler, HTTPServer
from pathlib import Path

import toml
from materialize.cargo import Workspace


Expand Down
4 changes: 1 addition & 3 deletions misc/python/materialize/cli/mzbench.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# mzbench.py -- script to run materialized benchmarks

import argparse
import collections
import csv
import itertools
import multiprocessing
Expand All @@ -20,7 +19,6 @@
import sys
import typing
import uuid
import webbrowser
from typing import Any, Dict, List, Optional, Tuple, Union


Expand Down Expand Up @@ -250,7 +248,7 @@ def main(args: argparse.Namespace) -> None:
"web",
f"perf-dash-web",
]
output = subprocess.check_output(web_command, stderr=subprocess.STDOUT)
subprocess.check_output(web_command, stderr=subprocess.STDOUT)
except (subprocess.CalledProcessError,) as e:
print(f"Failed to open browser to perf-dash:\n{e.output.decode()}")
raise
Expand Down
5 changes: 1 addition & 4 deletions misc/python/materialize/cli/mzcompose.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,13 @@
# mzcompose.py — runs Docker Compose with Materialize customizations.

import argparse
import json
import os
import subprocess
import sys
import webbrowser
from pathlib import Path
from typing import IO, List, Optional, Sequence, Text, Tuple
from typing import IO, List, NoReturn, Optional, Sequence, Text, Tuple

from materialize import errors, mzbuild, mzcompose, spawn, ui
from typing_extensions import NoReturn

announce = ui.speaker("==> ")
say = ui.speaker("")
Expand Down
4 changes: 2 additions & 2 deletions misc/python/materialize/cli/mzimage.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
import os
import sys
from pathlib import Path
from typing import Any, List
from typing import Any

from materialize import git, mzbuild, ui
from materialize import mzbuild, ui


def main() -> int:
Expand Down
1 change: 0 additions & 1 deletion misc/python/materialize/cli/scratch/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
# the Business Source License, use of this software will be governed
# by the Apache License, Version 2.0.

import argparse
import os

# Sane defaults for internal Materialize use in the scratch account
Expand Down
7 changes: 1 addition & 6 deletions misc/python/materialize/cli/scratch/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,12 @@
# by the Apache License, Version 2.0.

import argparse
import types
from typing import Callable, NamedTuple

from materialize.cli.scratch import create, mine

from materialize import errors
from materialize.cli.scratch import create, destroy, mine


def main() -> None:
from materialize.cli.scratch import create, destroy, mine

parser = argparse.ArgumentParser()
subparsers = parser.add_subparsers(dest="subcommand")
for name, configure, run in [
Expand Down
4 changes: 1 addition & 3 deletions misc/python/materialize/cli/scratch/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@

import argparse
import json
import os
import random
import sys
from datetime import datetime, timedelta, timezone
from typing import Any, Dict, List

import boto3
from materialize.cli.scratch import (
DEFAULT_INSTPROF_NAME,
DEFAULT_SG_ID,
Expand Down Expand Up @@ -89,7 +87,7 @@ def run(args: argparse.Namespace) -> None:
for obj in multi_json(sys.stdin.read())
]

nonce = "".join(random.choice("0123456789abcdef") for n in range(8))
nonce = "".join(random.choice("0123456789abcdef") for _ in range(8))

delete_after = int(datetime.now(timezone.utc).timestamp() + MAX_AGE.total_seconds())
instances = launch_cluster(
Expand Down
1 change: 1 addition & 0 deletions misc/python/materialize/cli/scratch/destroy.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import argparse

import boto3

from materialize.cli.scratch import check_required_vars
from materialize.scratch import print_instances

Expand Down
3 changes: 2 additions & 1 deletion misc/python/materialize/cli/scratch/mine.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
from typing import Callable

import boto3
from mypy_boto3_ec2.service_resource import Instance

from materialize.cli.scratch import check_required_vars
from materialize.scratch import launched_by, print_instances, tags, whoami
from mypy_boto3_ec2.service_resource import Instance


def configure_parser(parser: argparse.ArgumentParser) -> None:
Expand Down
4 changes: 2 additions & 2 deletions misc/python/materialize/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@

import subprocess
import sys
from functools import lru_cache, total_ordering
from functools import lru_cache
from pathlib import Path
from typing import List, NamedTuple, Optional, Set, Union
from typing import List, Optional, Set, Union

import semver.version

Expand Down
19 changes: 2 additions & 17 deletions misc/python/materialize/mzbuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
"""

import base64
import contextlib
import enum
import hashlib
import json
Expand All @@ -31,23 +30,9 @@
from functools import lru_cache
from pathlib import Path
from tempfile import TemporaryFile
from typing import (
IO,
Any,
Dict,
Iterable,
Iterator,
List,
Optional,
Sequence,
Set,
Union,
cast,
overload,
)
from typing import IO, Any, Dict, Iterable, Iterator, List, Sequence, Set

import yaml
from typing_extensions import Literal

from materialize import cargo, git, spawn, ui

Expand Down Expand Up @@ -677,7 +662,7 @@ def acquire(self, force_build: bool = False) -> None:
d.build()
else:
announce(f"Acquiring {spec}")
acquired_from = d.acquire()
d.acquire()
else:
announce(f"Already built {spec}")

Expand Down
Loading

0 comments on commit cdbcf54

Please sign in to comment.