Skip to content

Commit

Permalink
typing: Remove now-unnecessary conditional import.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Wyatt Hoodes authored and timabbott committed Jul 29, 2019
1 parent 865a720 commit a109508
Show file tree
Hide file tree
Showing 23 changed files with 25 additions and 116 deletions.
30 changes: 0 additions & 30 deletions docs/testing/mypy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions puppet/zulip/files/postgresql/pg_backup_and_purge
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 2 additions & 3 deletions puppet/zulip_ops/files/zulip-ec2-configure-interfaces
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 1 addition & 3 deletions scripts/lib/clean_emoji_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
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)
Expand Down
4 changes: 1 addition & 3 deletions scripts/lib/clean_node_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
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)
Expand Down
4 changes: 1 addition & 3 deletions scripts/lib/clean_venv_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
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)
Expand Down
5 changes: 1 addition & 4 deletions scripts/lib/hash_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
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]
Expand Down
5 changes: 1 addition & 4 deletions scripts/lib/node_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
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__))))
Expand Down
6 changes: 2 additions & 4 deletions scripts/lib/setup_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,15 @@
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"

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",
Expand Down
4 changes: 1 addition & 3 deletions scripts/lib/zulip_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
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")
Expand Down
5 changes: 1 addition & 4 deletions scripts/nagios/check-rabbitmq-consumers
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 1 addition & 3 deletions scripts/nagios/cron_file_helper.py
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
4 changes: 1 addition & 3 deletions scripts/purge-old-deployments
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions scripts/setup/generate_secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions scripts/setup/restore-backup
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions tools/diagnose
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 0 additions & 19 deletions tools/linter_lib/custom_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,21 +330,6 @@
'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',
Expand Down Expand Up @@ -798,10 +783,6 @@
'description': 'Linkified markdown URLs should use cleaner <http://example.com> 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/",
},
Expand Down
3 changes: 1 addition & 2 deletions zerver/lib/api_test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
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')
Expand Down

0 comments on commit a109508

Please sign in to comment.