diff --git a/.gitignore b/.gitignore index bdeb076b3..34fc5fa99 100644 --- a/.gitignore +++ b/.gitignore @@ -169,3 +169,6 @@ venv # tools tools/prometheus* + +# ray project files +project-id diff --git a/ci/travis/check_import_order.py b/ci/travis/check_import_order.py index 216f73af9..863670163 100644 --- a/ci/travis/check_import_order.py +++ b/ci/travis/check_import_order.py @@ -1,7 +1,16 @@ +""" +This script ensures python files conform to ray's import ordering rules. +In particular, we make sure psutil and setproctitle is imported _after_ +importing ray due to our bundling of the two libraries. + +Usage: +$ python check_import_order.py SOURCE_DIR -s SKIP_DIR +some/file/path.py:23 import psutil without explicitly import ray before it. +""" + import argparse import re import sys -import tempfile from pathlib import Path exit_with_error = False @@ -17,7 +26,17 @@ def check_import(file): with open(file) as f: for i, line in enumerate(f): for check in check_to_lines.keys(): - if re.match(r"\s+" + check + r"(\s|$)", line): + # This regex will match the following case + # - the string itself: `import psutil` + # - white space/indentation + the string:` import psutil` + # - the string and arbitrary whitespace: `import psutil ` + # - the string and the noqa flag to silent pylint + # `import psutil # noqa F401 import-ordering` + # It will not match the following + # - submodule import: `import ray.constants as ray_constants` + # - submodule import: `from ray import xyz` + if re.search(r"^\s*" + check + r"(\s*|\s+# noqa F401.*)$", + line): check_to_lines[check] = i for import_lib in ["import psutil", "import setproctitle"]: @@ -27,64 +46,17 @@ def check_import(file): if import_ray_line == -1 or import_ray_line > import_psutil_line: print( "{}:{}".format(str(file), import_psutil_line + 1), - "{} without explicit import ray before it.".format( + "{} without explicitly import ray before it.".format( import_lib)) global exit_with_error exit_with_error = True -# Run the test with pytest file_name -def test_check_import(): - global exit_with_error - _, path = tempfile.mkstemp() - - with open(path, "w") as f: - f.write(""" - import psutil - import ray - """) - check_import(path) - assert exit_with_error - exit_with_error = False - - with open(path, "w") as f: - f.write(""" - import psutil - """) - check_import(path) - assert exit_with_error - exit_with_error = False - - with open(path, "w") as f: - f.write(""" - import random_lib - """) - check_import(path) - assert not exit_with_error - exit_with_error = False - - with open(path, "w") as f: - f.write(""" - import setproctitle - import ray - """) - check_import(path) - assert exit_with_error - exit_with_error = False - - with open(path, "w") as f: - f.write(""" - import ray - import psutil - """) - check_import(path) - assert not exit_with_error - exit_with_error = False - - if __name__ == "__main__": parser = argparse.ArgumentParser() parser.add_argument("path", help="File path to check. e.g. '.' or './src'") + # TODO(simon): For the future, consider adding a feature to explicitly + # white-list the path instead of skipping them. parser.add_argument( "-s", "--skip", action="append", help="Skip certian directory") args = parser.parse_args() diff --git a/ci/travis/format.sh b/ci/travis/format.sh index db75885e1..94aa47eae 100755 --- a/ci/travis/format.sh +++ b/ci/travis/format.sh @@ -145,7 +145,7 @@ fi # Ensure import ordering # Make sure that for every import psutil; import setpproctitle # There's a import ray above it. -python ci/travis/check_import_order.py . -s ci -s python/ray/pyarrow_files +python ci/travis/check_import_order.py . -s ci -s python/ray/pyarrow_files -s python/ray/thirdparty_files -s python/build if ! git diff --quiet &>/dev/null; then echo 'Reformatted changed files. Please review and stage the changes.' diff --git a/ci/travis/install-dependencies.sh b/ci/travis/install-dependencies.sh index 99d26cb2a..3da738850 100755 --- a/ci/travis/install-dependencies.sh +++ b/ci/travis/install-dependencies.sh @@ -25,7 +25,7 @@ if [[ "$PYTHON" == "3.5" ]] && [[ "$platform" == "linux" ]]; then bash miniconda.sh -b -p $HOME/miniconda export PATH="$HOME/miniconda/bin:$PATH" pip install -q scipy tensorflow cython==0.29.0 gym opencv-python-headless pyyaml pandas==0.24.2 requests \ - feather-format lxml openpyxl xlrd py-spy setproctitle pytest-timeout networkx tabulate psutil aiohttp \ + feather-format lxml openpyxl xlrd py-spy pytest-timeout networkx tabulate aiohttp \ uvicorn dataclasses pygments werkzeug kubernetes flask grpcio pytest-sugar pytest-rerunfailures pytest-asyncio \ blist torch torchvision scikit-learn elif [[ "$PYTHON" == "3.5" ]] && [[ "$platform" == "macosx" ]]; then @@ -34,7 +34,7 @@ elif [[ "$PYTHON" == "3.5" ]] && [[ "$platform" == "macosx" ]]; then bash miniconda.sh -b -p $HOME/miniconda export PATH="$HOME/miniconda/bin:$PATH" pip install -q cython==0.29.0 tensorflow gym opencv-python-headless pyyaml pandas==0.24.2 requests \ - feather-format lxml openpyxl xlrd py-spy setproctitle pytest-timeout networkx tabulate psutil aiohttp \ + feather-format lxml openpyxl xlrd py-spy pytest-timeout networkx tabulate aiohttp \ uvicorn dataclasses pygments werkzeug kubernetes flask grpcio pytest-sugar pytest-rerunfailures pytest-asyncio \ blist torch torchvision scikit-learn elif [[ "$LINT" == "1" ]]; then diff --git a/doc/requirements-doc.txt b/doc/requirements-doc.txt index e0b0c7eeb..8a2cb75fd 100644 --- a/doc/requirements-doc.txt +++ b/doc/requirements-doc.txt @@ -12,6 +12,7 @@ opencv-python-headless pandas pyarrow pygments +psutil pyyaml recommonmark redis diff --git a/python/ray/test_utils.py b/python/ray/test_utils.py index 5f061310a..6d0788225 100644 --- a/python/ray/test_utils.py +++ b/python/ray/test_utils.py @@ -6,10 +6,10 @@ import sys import tempfile import time -import psutil - import ray +import psutil # We must import psutil after ray because we bundle it with ray. + class RayTestTimeoutException(Exception): """Exception used to identify timeouts from test utilities.""" diff --git a/python/ray/tests/test_metrics.py b/python/ray/tests/test_metrics.py index 66d00e1a7..001446e4b 100644 --- a/python/ray/tests/test_metrics.py +++ b/python/ray/tests/test_metrics.py @@ -1,7 +1,6 @@ import os import json import grpc -import psutil import pytest import requests import time @@ -14,6 +13,8 @@ from ray.core.generated import reporter_pb2_grpc from ray.test_utils import (RayTestTimeoutException, wait_until_succeeded_without_exception) +import psutil # We must import psutil after ray because we bundle it with ray. + def test_worker_stats(shutdown_only): addresses = ray.init(num_cpus=1, include_webui=True)