From a109508e3463dc3e3d8ece332f42a4ca5e4f1adc Mon Sep 17 00:00:00 2001 From: Wyatt Hoodes Date: Tue, 23 Jul 2019 11:58:11 -1000 Subject: [PATCH] typing: Remove now-unnecessary conditional import. As a result of dropping support for trusty, we can remove our old pattern of putting `if False` before importing the typing module, which was essential for Python 3.4 support, but not required and maybe harmful on newer versions. cron_file_helper check_rabbitmq_consumers hash_reqs check_zephyr_mirror check_personal_zephyr_mirrors check_cron_file zulip_tools check_postgres_replication_lag api_test_helpers purge-old-deployments setup_venv node_cache clean_venv_cache clean_node_cache clean_emoji_cache pg_backup_and_purge restore-backup generate_secrets zulip-ec2-configure-interfaces diagnose check_user_zephyr_mirror_liveness --- docs/testing/mypy.md | 30 ------------------- .../zulip_app_frontend/check_cron_file | 5 +--- .../check_postgres_replication_lag | 5 ++-- .../files/postgresql/pg_backup_and_purge | 3 +- .../check_personal_zephyr_mirrors | 5 +--- .../check_user_zephyr_mirror_liveness | 4 +-- .../zulip_zephyr_mirror/check_zephyr_mirror | 5 +--- .../files/zulip-ec2-configure-interfaces | 5 ++-- scripts/lib/clean_emoji_cache.py | 4 +-- scripts/lib/clean_node_cache.py | 4 +-- scripts/lib/clean_venv_cache.py | 4 +-- scripts/lib/hash_reqs.py | 5 +--- scripts/lib/node_cache.py | 5 +--- scripts/lib/setup_venv.py | 6 ++-- scripts/lib/zulip_tools.py | 4 +-- scripts/nagios/check-rabbitmq-consumers | 5 +--- scripts/nagios/cron_file_helper.py | 4 +-- scripts/purge-old-deployments | 4 +-- scripts/setup/generate_secrets.py | 5 ++-- scripts/setup/restore-backup | 3 +- tools/diagnose | 4 +-- tools/linter_lib/custom_check.py | 19 ------------ zerver/lib/api_test_helpers.py | 3 +- 23 files changed, 25 insertions(+), 116 deletions(-) diff --git a/docs/testing/mypy.md b/docs/testing/mypy.md index 0c486e1940..9070393505 100644 --- a/docs/testing/mypy.md +++ b/docs/testing/mypy.md @@ -95,36 +95,6 @@ an `Any` or `# type: ignore` so you're not blocked waiting for help, add a `# TODO: ` comment so it doesn't get forgotten in code review, and ask for help in chat.zulip.org. -## mypy in production scripts - -While in most of the Zulip codebase, we can consistently use the -`typing` module (Part of the standard library in Python 3.5, but -present as an installable module with older Python), in our installer -and other production scripts that might run outside a Zulip -virtualenv, we cannot rely on the `typing` module being present on the -system. - -To solve this problem, we use the following (semi-standard in the mypy -community) hack in those scripts: - -``` -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import List -``` - -and then use the Python 2 style type comment syntax for annotating -those files. This way, the Python interpreters for Python 2.7 and 3.4 -will ignore this line, and thus not crash. But we can still get all -the benefits of type annotations in that codebase, since the `mypy` -type checker ignores the `if False` and thus still is able to -type-check the file using those imports. - -The exception to this rule is that any scripts which use -`setup_path_on_import` before they import from the `typing` module are -safe. These, we generally declare in the relevant exclude line in -`tools/linter_lib/custom_check.py` - ## mypy stubs for third-party modules. For the Python standard library and some popular third-party modules, diff --git a/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_cron_file b/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_cron_file index 123fe8f257..b0f165f10d 100755 --- a/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_cron_file +++ b/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_cron_file @@ -4,10 +4,7 @@ file output by the cron job is correct. """ import sys import time - -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Tuple +from typing import Tuple def nagios_from_file(results_file: str, max_time_diff: int=60 * 2) -> 'Tuple[int, str]': """Returns a nagios-appropriate string and return code obtained by diff --git a/puppet/zulip/files/nagios_plugins/zulip_nagios_server/check_postgres_replication_lag b/puppet/zulip/files/nagios_plugins/zulip_nagios_server/check_postgres_replication_lag index 3a053b603e..58eb27b6ba 100755 --- a/puppet/zulip/files/nagios_plugins/zulip_nagios_server/check_postgres_replication_lag +++ b/puppet/zulip/files/nagios_plugins/zulip_nagios_server/check_postgres_replication_lag @@ -4,13 +4,12 @@ Nagios plugin to check the difference between the primary and secondary Postgres servers' xlog location. """ -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from mypy_extensions import NoReturn import subprocess import re +from mypy_extensions import NoReturn + states = { "OK": 0, "WARNING": 1, diff --git a/puppet/zulip/files/postgresql/pg_backup_and_purge b/puppet/zulip/files/postgresql/pg_backup_and_purge index d823e5964e..613c951aa6 100755 --- a/puppet/zulip/files/postgresql/pg_backup_and_purge +++ b/puppet/zulip/files/postgresql/pg_backup_and_purge @@ -10,8 +10,7 @@ import dateutil.parser import pytz import time from datetime import datetime, timedelta -if False: - from typing import Dict, List +from typing import Dict, List logging.Formatter.converter = time.gmtime logging.basicConfig(format="%(asctime)s %(levelname)s: %(message)s") diff --git a/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_personal_zephyr_mirrors b/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_personal_zephyr_mirrors index c8e78d84d3..dd5c6481d3 100755 --- a/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_personal_zephyr_mirrors +++ b/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_personal_zephyr_mirrors @@ -8,10 +8,7 @@ This script works by just monitoring the files under mirrors when they receive the messages sent every minute by /etc/cron.d/test_zephyr_personal_mirrors """ -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Dict - +from typing import Dict import os import time diff --git a/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_user_zephyr_mirror_liveness b/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_user_zephyr_mirror_liveness index 2ac51e77c4..81ba42d7ea 100755 --- a/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_user_zephyr_mirror_liveness +++ b/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_user_zephyr_mirror_liveness @@ -24,9 +24,7 @@ django.setup() from zerver.models import UserActivity -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Any, Dict, Set, Optional +from typing import Any, Dict, Set, Optional states = { "OK": 0, diff --git a/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_zephyr_mirror b/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_zephyr_mirror index 69569d9360..ea9a9adea6 100755 --- a/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_zephyr_mirror +++ b/puppet/zulip_ops/files/nagios_plugins/zulip_zephyr_mirror/check_zephyr_mirror @@ -9,10 +9,7 @@ run out of cron. See puppet/zulip_ops/files/cron.d/zephyr-mirror for the crontab details. """ -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Dict - +from typing import Dict import os import time diff --git a/puppet/zulip_ops/files/zulip-ec2-configure-interfaces b/puppet/zulip_ops/files/zulip-ec2-configure-interfaces index 8e2e08cd68..ca6d7fc75b 100755 --- a/puppet/zulip_ops/files/zulip-ec2-configure-interfaces +++ b/puppet/zulip_ops/files/zulip-ec2-configure-interfaces @@ -50,9 +50,8 @@ import subprocess import boto.utils import netifaces -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Optional + +from typing import Optional def address_of(device_id): # type: (int) -> Optional[str] diff --git a/scripts/lib/clean_emoji_cache.py b/scripts/lib/clean_emoji_cache.py index 04f81486a7..74285f32a3 100755 --- a/scripts/lib/clean_emoji_cache.py +++ b/scripts/lib/clean_emoji_cache.py @@ -3,9 +3,7 @@ import argparse import os import sys -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Set +from typing import Set ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) sys.path.append(ZULIP_PATH) diff --git a/scripts/lib/clean_node_cache.py b/scripts/lib/clean_node_cache.py index 4593e2e38d..c7446ed770 100755 --- a/scripts/lib/clean_node_cache.py +++ b/scripts/lib/clean_node_cache.py @@ -4,9 +4,7 @@ import os import subprocess import sys -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Set +from typing import Set ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) sys.path.append(ZULIP_PATH) diff --git a/scripts/lib/clean_venv_cache.py b/scripts/lib/clean_venv_cache.py index 44f139fab4..a0f75f0512 100755 --- a/scripts/lib/clean_venv_cache.py +++ b/scripts/lib/clean_venv_cache.py @@ -3,9 +3,7 @@ import argparse import os import sys -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Set +from typing import Set ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) sys.path.append(ZULIP_PATH) diff --git a/scripts/lib/hash_reqs.py b/scripts/lib/hash_reqs.py index 875596256f..e7de7b4cb5 100755 --- a/scripts/lib/hash_reqs.py +++ b/scripts/lib/hash_reqs.py @@ -3,10 +3,7 @@ import os import sys import argparse import hashlib - -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Iterable, List, MutableSet +from typing import Iterable, List, MutableSet def expand_reqs_helper(fpath, visited): # type: (str, MutableSet[str]) -> List[str] diff --git a/scripts/lib/node_cache.py b/scripts/lib/node_cache.py index c2a17817b0..94350bc594 100644 --- a/scripts/lib/node_cache.py +++ b/scripts/lib/node_cache.py @@ -3,10 +3,7 @@ import os import hashlib import json -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Optional, List, IO, Any - +from typing import Optional, List, IO, Any from scripts.lib.zulip_tools import subprocess_text_output, run ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) diff --git a/scripts/lib/setup_venv.py b/scripts/lib/setup_venv.py index 92c8c2d55e..5ad6808eb0 100644 --- a/scripts/lib/setup_venv.py +++ b/scripts/lib/setup_venv.py @@ -7,6 +7,8 @@ import sys from scripts.lib.zulip_tools import run, run_as_root, ENDC, WARNING from scripts.lib.hash_reqs import expand_reqs +from typing import List, Optional, Tuple, Set + ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) VENV_CACHE_PATH = "/srv/zulip-venv-cache" @@ -14,10 +16,6 @@ if 'TRAVIS' in os.environ: # In Travis CI, we don't have root access VENV_CACHE_PATH = "/home/travis/zulip-venv-cache" -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import List, Optional, Tuple, Set - VENV_DEPENDENCIES = [ "build-essential", "libffi-dev", diff --git a/scripts/lib/zulip_tools.py b/scripts/lib/zulip_tools.py index 07574a8015..2f8d3acdc6 100755 --- a/scripts/lib/zulip_tools.py +++ b/scripts/lib/zulip_tools.py @@ -16,9 +16,7 @@ import json import uuid import configparser -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Sequence, Set, Any, Dict, List +from typing import Sequence, Set, Any, Dict, List DEPLOYMENTS_DIR = "/home/zulip/deployments" LOCK_DIR = os.path.join(DEPLOYMENTS_DIR, "lock") diff --git a/scripts/nagios/check-rabbitmq-consumers b/scripts/nagios/check-rabbitmq-consumers index ae3dd64215..092168350c 100755 --- a/scripts/nagios/check-rabbitmq-consumers +++ b/scripts/nagios/check-rabbitmq-consumers @@ -7,10 +7,7 @@ import configparser from collections import defaultdict import os import subprocess - -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Dict +from typing import Dict states = { 0: "OK", diff --git a/scripts/nagios/cron_file_helper.py b/scripts/nagios/cron_file_helper.py index 268274050a..540267e568 100644 --- a/scripts/nagios/cron_file_helper.py +++ b/scripts/nagios/cron_file_helper.py @@ -1,8 +1,6 @@ import time -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Tuple +from typing import Tuple def nagios_from_file(results_file): # type: (str) -> Tuple[int, str] diff --git a/scripts/purge-old-deployments b/scripts/purge-old-deployments index c0622709cc..2c65cd3ad1 100755 --- a/scripts/purge-old-deployments +++ b/scripts/purge-old-deployments @@ -4,9 +4,7 @@ import os import subprocess import sys -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Set +from typing import Set ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.append(ZULIP_PATH) diff --git a/scripts/setup/generate_secrets.py b/scripts/setup/generate_secrets.py index 5f8c2aa0ae..f9ed56dbd7 100755 --- a/scripts/setup/generate_secrets.py +++ b/scripts/setup/generate_secrets.py @@ -3,9 +3,8 @@ import sys import os -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Dict, List + +from typing import Dict, List BASE_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) sys.path.append(BASE_DIR) diff --git a/scripts/setup/restore-backup b/scripts/setup/restore-backup index f6814677d4..c2c66d7dfe 100755 --- a/scripts/setup/restore-backup +++ b/scripts/setup/restore-backup @@ -7,8 +7,7 @@ import subprocess import sys import tempfile -if False: - from typing import IO +from typing import IO BASE_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) sys.path.append(BASE_DIR) diff --git a/tools/diagnose b/tools/diagnose index 66ce32627d..37c2aa75d3 100755 --- a/tools/diagnose +++ b/tools/diagnose @@ -6,9 +6,7 @@ import shlex import sys import subprocess -if False: - # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Callable, List +from typing import Callable, List TOOLS_DIR = os.path.dirname(__file__) ROOT_DIR = os.path.dirname(TOOLS_DIR) diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index 13559a858d..08660e08c8 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -330,21 +330,6 @@ python_rules = RuleList( 'description': 'Most scripts are intended to run on systems without sudo.', 'good_lines': ['subprocess.check_call(["ls"])'], 'bad_lines': ['subprocess.check_call(["sudo", "ls"])']}, - {'pattern': '^from typing import', - 'strip': '\n', - 'include_only': set([ - 'scripts/', - 'puppet/', - ]), - 'exclude': set([ - # Not important, but should fix - 'scripts/lib/process-mobile-i18n', - # Uses setup_path_on_import before importing. - 'puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_send_receive_time', - ]), - 'description': 'For scripts run as part of installer, cannot rely on typing existing; use `if False` workaround.', - 'good_lines': [' from typing import List'], - 'bad_lines': ['from typing import List']}, {'pattern': 'django.utils.translation', 'include_only': set(['test/', 'zerver/views/development/']), 'description': 'Test strings should not be tagged for translation', @@ -798,10 +783,6 @@ markdown_rules = RuleList( 'description': 'Linkified markdown URLs should use cleaner syntax.'}, {'pattern': 'https://zulip.readthedocs.io/en/latest/[a-zA-Z0-9]', 'exclude': ['docs/overview/contributing.md', 'docs/overview/readme.md', 'docs/README.md'], - 'exclude_line': set([ - ('docs/testing/mypy.md', - '# See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts') - ]), 'include_only': set(['docs/']), 'description': "Use relative links (../foo/bar.html) to other documents in docs/", }, diff --git a/zerver/lib/api_test_helpers.py b/zerver/lib/api_test_helpers.py index 10951b0802..ebbc0c05d1 100644 --- a/zerver/lib/api_test_helpers.py +++ b/zerver/lib/api_test_helpers.py @@ -6,8 +6,7 @@ import os from zerver.lib import mdiff from zerver.lib.openapi import validate_against_openapi_schema -if False: - from zulip import Client +from zulip import Client ZULIP_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) FIXTURE_PATH = os.path.join(ZULIP_DIR, 'templates', 'zerver', 'api', 'fixtures.json')