You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2015/11/24 01:49:48 UTC
aurora git commit: Replace Twitter checkstyle with pants checkstyle.
Repository: aurora
Updated Branches:
refs/heads/master a0e128d2f -> cd4f7ad6d
Replace Twitter checkstyle with pants checkstyle.
This installs the ~newly split off pants python checks contrib plugin.
Release notes for that were here:
https://pypi.python.org/pypi/pantsbuild.pants/0.0.58
The plugin provides both python checkstyle (`compile.pythonstyle`), and
a python eval task (`compile.python-eval`). The `python-eval` is turned
off since at least one of the Aurora python targets has files that have
side-effects upon import (a repl is started).
Now style checks run before compile (and thus before tests) and they
benefit from fingerprinting; ie: if you test your changes, those tests
will run style checks and when you go to commit, those checks will not
be re-run by the commit hook (although files you did not test will still
need to be checked).
A few production files were fixed up according to style failures coming
from:
+ no space after comment opening '#'
+ unused variables
+ mis-aligned hanging closing parens.
Bugs closed: AURORA-1532
Reviewed at https://reviews.apache.org/r/40310/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/cd4f7ad6
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/cd4f7ad6
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/cd4f7ad6
Branch: refs/heads/master
Commit: cd4f7ad6d52bde51e3adbd27139c88ac4bfcf73e
Parents: a0e128d
Author: John Sirois <jo...@gmail.com>
Authored: Mon Nov 23 16:49:48 2015 -0800
Committer: Bill Farner <wf...@apache.org>
Committed: Mon Nov 23 16:49:48 2015 -0800
----------------------------------------------------------------------
build-support/hooks/pre-commit | 3 +-
build-support/jenkins/build.sh | 8 ++--
build-support/python/checkstyle | 34 -------------
build-support/python/checkstyle-check | 6 +--
pants.ini | 50 ++++++++++++++++++++
.../python/apache/aurora/admin/maintenance.py | 2 +-
.../python/apache/aurora/client/api/__init__.py | 7 ++-
.../python/apache/aurora/client/cli/client.py | 2 +-
.../python/apache/aurora/client/cli/cron.py | 2 +-
src/main/python/apache/thermos/core/process.py | 6 +--
.../monitoring/process_collector_psutil.py | 1 -
.../python/apache/aurora/admin/test_admin.py | 12 ++---
src/test/python/apache/aurora/admin/util.py | 2 +-
.../apache/aurora/client/cli/test_task.py | 3 +-
14 files changed, 77 insertions(+), 61 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/build-support/hooks/pre-commit
----------------------------------------------------------------------
diff --git a/build-support/hooks/pre-commit b/build-support/hooks/pre-commit
index 619fa9e..51d97d3 100755
--- a/build-support/hooks/pre-commit
+++ b/build-support/hooks/pre-commit
@@ -20,7 +20,6 @@ if [ $SKIP_AURORA_HOOKS ]; then
fi
HERE=$(cd `dirname "${BASH_SOURCE[0]}"` && pwd)
-TARGET=$HERE/../../src
echo "Performing Python import order check."
if ! $HERE/../../build-support/python/isort-check >&/dev/null; then
@@ -43,7 +42,7 @@ if ! $HERE/../../build-support/python/checkstyle-check $TARGET >&/dev/null; then
echo ""
echo "** PYTHON CHECKSTYLE FAILED"
echo "*"
- echo "* For more information please run: build-support/python/checkstyle-check $TARGET"
+ echo "* For more information please run: build-support/python/checkstyle-check"
echo "*"
echo "**"
echo ""
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/build-support/jenkins/build.sh
----------------------------------------------------------------------
diff --git a/build-support/jenkins/build.sh b/build-support/jenkins/build.sh
index 41a3921..1de1446 100755
--- a/build-support/jenkins/build.sh
+++ b/build-support/jenkins/build.sh
@@ -20,12 +20,12 @@ date
# Run all Java tests
./gradlew -Pq clean build
-# Run Python style checks
+# Run Python import ordering check
./build-support/python/isort-check
-./build-support/python/checkstyle-check src
-# Run all Python tests
-./pants test.pytest --junit-xml-dir="$PWD/dist/test-results" src/test/python:: -- -v
+# Run remaining Python style checks and all tests
+./pants test.pytest --junit-xml-dir="$PWD/dist/test-results" \
+ src/{main,test}/python:: -- -v
# Ensure we can build python sdists (AURORA-1174)
./build-support/release/make-python-sdists
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/build-support/python/checkstyle
----------------------------------------------------------------------
diff --git a/build-support/python/checkstyle b/build-support/python/checkstyle
deleted file mode 100755
index 61acc22..0000000
--- a/build-support/python/checkstyle
+++ /dev/null
@@ -1,34 +0,0 @@
-#!/usr/bin/env bash
-#
-# Copyright 2014 Apache Software Foundation
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-# Wrapper script for running isort
-set -e
-
-HERE=$(cd `dirname "${BASH_SOURCE[0]}"` && pwd)
-CHECKSTYLE_VERSION=0.1.0
-
-if ! [ -f "$HERE/checkstyle.venv/BOOTSTRAPPED" ] || \
- [ x`cat "$HERE/checkstyle.venv/BOOTSTRAPPED"` != x$CHECKSTYLE_VERSION ]; then
- echo Bootstrapping checkstyle @ $CHECKSTYLE_VERSION
- rm -fr "$HERE/checkstyle.venv"
- "$HERE/../virtualenv" "$HERE/checkstyle.venv"
- source "$HERE/checkstyle.venv/bin/activate"
- python -m pip install "twitter.checkstyle==$CHECKSTYLE_VERSION"
- echo $CHECKSTYLE_VERSION > "$HERE/checkstyle.venv/BOOTSTRAPPED"
-fi
-
-source "$HERE/checkstyle.venv/bin/activate"
-exec twitterstyle "$@"
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/build-support/python/checkstyle-check
----------------------------------------------------------------------
diff --git a/build-support/python/checkstyle-check b/build-support/python/checkstyle-check
index b2bfc5d..0c88002 100755
--- a/build-support/python/checkstyle-check
+++ b/build-support/python/checkstyle-check
@@ -18,6 +18,6 @@ set -e
HERE=$(cd `dirname "${BASH_SOURCE[0]}"` && pwd)
-# Run checkstyle but skip the ImportOrder check as that is enforced by
-# isort.
-$HERE/checkstyle -n ImportOrder "$@"
+# TODO(John Sirois): Consider using `./pants changed` as a feed for this check.
+# We need not be checking the whole repo on evey commit.
+$HERE/../../pants compile.pythonstyle src/{main,test}/python::
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/pants.ini
----------------------------------------------------------------------
diff --git a/pants.ini b/pants.ini
index d58908c..1e19d9a 100644
--- a/pants.ini
+++ b/pants.ini
@@ -14,6 +14,10 @@
[DEFAULT]
pants_version: 0.0.59
+plugins: [
+ 'pantsbuild.pants.contrib.python.checks==%(pants_version)s',
+ ]
+
[thrift-binary]
# Pants 0.0.57 defaults to 0.9.2, we want to stay pinned down to 0.9.1 for now.
@@ -29,3 +33,49 @@ interpreter_requirement: CPython>=2.7,<3
# isolates one pytest session in one chroot per test target. More info here:
# http://pantsbuild.github.io/options_reference.html#group_testpytest
fast: False
+
+
+# We have some modules that have side-effects upon import, including starting a repl, so we can't
+# use python-eval to validate our BUILD deps currently.
+[compile.python-eval]
+skip: True
+
+
+# We use isort for this.
+[pycheck-import-order]
+skip: True
+
+
+[pycheck-pep8]
+# Code reference is here: http://pep8.readthedocs.org/en/latest/intro.html#error-codes
+ignore: [
+ # Aurora custom ignores:
+ 'E114', # indentation is not a multiple of four (comment)
+ 'E116', # unexpected indentation (comment)
+ 'E122', # continuation line missing indentation or outdented
+ 'E126', # continuation line over-indented for hanging indent
+ 'E129', # visually indented line with same indent as next logical line
+ 'E131', # continuation line unaligned for hanging indent
+ 'E731', # do not assign a lambda expression, use a def
+ 'W503', # line break before binary operator
+
+ # These are a subset of the standard ignores pre-packaged for pycheck-pep8/pep8, but we need to
+ # repeat here since we add our own above:
+ 'E111', # indentation is not a multiple of four
+ 'E121', # continuation line under-indented for hanging indent
+ 'E125', # continuation line with same indent as next logical line
+ 'E127', # continuation line over-indented for visual indent
+ 'E128', # continuation line under-indented for visual indent
+ 'E301', # expected 1 blank line, found 0 # We allow consecutive exception declarations.
+ 'E401', # multiple imports on one line
+ 'E701', # multiple statements on one line (colon) # We allow: `class Exc(Exception): pass`.
+ ]
+
+
+# We disable the class factoring check since it flags calls to superclass constructors from nested
+# classes. We do this commonly enough in nested exception classes.
+# The error looks like so:
+# T800 Instead of Context.CommandError use self.CommandError or cls.CommandError with
+# instancemethods and classmethods respectively.
+[pycheck-class-factoring]
+skip: True
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/main/python/apache/aurora/admin/maintenance.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/admin/maintenance.py b/src/main/python/apache/aurora/admin/maintenance.py
index 6d94c92..bf44651 100644
--- a/src/main/python/apache/aurora/admin/maintenance.py
+++ b/src/main/python/apache/aurora/admin/maintenance.py
@@ -32,7 +32,7 @@ from .admin_util import (
from .host_maintenance import HostMaintenance
-#TODO(maxim): merge with admin.py commands.
+# TODO(maxim): merge with admin.py commands.
@app.command
@app.command_option(FILENAME_OPTION)
@app.command_option(HOSTS_OPTION)
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/main/python/apache/aurora/client/api/__init__.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/api/__init__.py b/src/main/python/apache/aurora/client/api/__init__.py
index 6f07a30..1514394 100644
--- a/src/main/python/apache/aurora/client/api/__init__.py
+++ b/src/main/python/apache/aurora/client/api/__init__.py
@@ -280,8 +280,11 @@ class AuroraClientAPI(object):
"""
self._assert_valid_job_key(job_key)
- return Restarter(job_key, updater_config, health_check_interval_seconds, self._scheduler_proxy
- ).restart(instances)
+ return Restarter(
+ job_key,
+ updater_config,
+ health_check_interval_seconds,
+ self._scheduler_proxy).restart(instances)
def start_maintenance(self, hosts):
log.info("Starting maintenance for: %s" % hosts.hostNames)
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/main/python/apache/aurora/client/cli/client.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/client.py b/src/main/python/apache/aurora/client/cli/client.py
index 297fb58..fa0c264 100644
--- a/src/main/python/apache/aurora/client/cli/client.py
+++ b/src/main/python/apache/aurora/client/cli/client.py
@@ -36,7 +36,7 @@ class AuroraLogConfigurationPlugin(ConfigurationPlugin):
]
def before_dispatch(self, raw_args):
- #TODO(zmanji): Consider raising the default log level to WARN.
+ # TODO(zmanji): Consider raising the default log level to WARN.
loglevel = logging.INFO
for arg in raw_args:
if arg == "--verbose" or arg == "-v":
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/main/python/apache/aurora/client/cli/cron.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/cron.py b/src/main/python/apache/aurora/client/cli/cron.py
index 6376fd0..33087f1 100644
--- a/src/main/python/apache/aurora/client/cli/cron.py
+++ b/src/main/python/apache/aurora/client/cli/cron.py
@@ -130,7 +130,7 @@ class Show(Verb):
return [JOBSPEC_ARGUMENT]
def execute(self, context):
- #TODO(mchucarroll): do we want to support wildcards here?
+ # TODO(mchucarroll): do we want to support wildcards here?
jobkey = context.options.jobspec
api = context.get_api(jobkey.cluster)
resp = api.get_jobs(jobkey.role)
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/main/python/apache/thermos/core/process.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/thermos/core/process.py b/src/main/python/apache/thermos/core/process.py
index fe95cb3..f214bcc 100644
--- a/src/main/python/apache/thermos/core/process.py
+++ b/src/main/python/apache/thermos/core/process.py
@@ -205,13 +205,13 @@ class ProcessBase(object):
if self._user:
if user != current_user and os.geteuid() != 0:
raise self.PermissionError('Must be root to run processes as other users!')
- uid, gid = user.pw_uid, user.pw_gid
self._fork_time = self._platform.clock().time()
self._setup_ckpt()
self._stdout = safe_open(self._pathspec.with_filename('stdout').getpath('process_logdir'), "a")
self._stderr = safe_open(self._pathspec.with_filename('stderr').getpath('process_logdir'), "a")
- os.chown(self._stdout.name, user.pw_uid, user.pw_gid)
- os.chown(self._stderr.name, user.pw_uid, user.pw_gid)
+ uid, gid = user.pw_uid, user.pw_gid
+ os.chown(self._stdout.name, uid, gid)
+ os.chown(self._stderr.name, uid, gid)
def _finalize_fork(self):
self._write_initial_update()
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/main/python/apache/thermos/monitoring/process_collector_psutil.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/thermos/monitoring/process_collector_psutil.py b/src/main/python/apache/thermos/monitoring/process_collector_psutil.py
index f1ec5a9..bb7c902 100644
--- a/src/main/python/apache/thermos/monitoring/process_collector_psutil.py
+++ b/src/main/python/apache/thermos/monitoring/process_collector_psutil.py
@@ -61,7 +61,6 @@ class ProcessTreeCollector(object):
Returns None: result is stored in self.value
"""
try:
- last_sample, last_stamp = self._sample, self._stamp
if self._process is None:
self._process = Process(self._pid)
parent = self._process
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/test/python/apache/aurora/admin/test_admin.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/admin/test_admin.py b/src/test/python/apache/aurora/admin/test_admin.py
index 8e204ab..e8da335 100644
--- a/src/test/python/apache/aurora/admin/test_admin.py
+++ b/src/test/python/apache/aurora/admin/test_admin.py
@@ -155,9 +155,9 @@ class TestIncreaseQuotaCommand(AuroraClientCommandTest):
increase_quota([self.TEST_CLUSTER, role, '4.0', '1MB', '1MB'])
api.set_quota.assert_called_with(role, 24.0, 4001, 6001)
- assert type(api.set_quota.call_args[0][1]) == type(float())
- assert type(api.set_quota.call_args[0][2]) == type(int())
- assert type(api.set_quota.call_args[0][3]) == type(int())
+ assert isinstance(api.set_quota.call_args[0][1], float)
+ assert isinstance(api.set_quota.call_args[0][2], int)
+ assert isinstance(api.set_quota.call_args[0][3], int)
class TestSetQuotaCommand(AuroraClientCommandTest):
@@ -187,9 +187,9 @@ class TestSetQuotaCommand(AuroraClientCommandTest):
set_quota([self.TEST_CLUSTER, role, '4.0', '10MB', '10MB'])
api.set_quota.assert_called_with(role, 4.0, 10, 10)
- assert type(api.set_quota.call_args[0][1]) == type(float())
- assert type(api.set_quota.call_args[0][2]) == type(int())
- assert type(api.set_quota.call_args[0][3]) == type(int())
+ assert isinstance(api.set_quota.call_args[0][1], float)
+ assert isinstance(api.set_quota.call_args[0][2], int)
+ assert isinstance(api.set_quota.call_args[0][3], int)
class TestGetLocksCommand(AuroraClientCommandTest):
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/test/python/apache/aurora/admin/util.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/admin/util.py b/src/test/python/apache/aurora/admin/util.py
index 3570407..d0a915c 100644
--- a/src/test/python/apache/aurora/admin/util.py
+++ b/src/test/python/apache/aurora/admin/util.py
@@ -76,7 +76,7 @@ class AuroraClientCommandTest(unittest.TestCase):
hosts[hostname].append(JobUpTimeDetails(job, predicted, safe, safe_in))
return [hosts]
- #TODO(wfarner): Remove this, force tests to call out their flags.
+ # TODO(wfarner): Remove this, force tests to call out their flags.
@classmethod
def setup_mock_options(cls):
mock_options = create_autospec(spec=['verbosity'], instance=True)
http://git-wip-us.apache.org/repos/asf/aurora/blob/cd4f7ad6/src/test/python/apache/aurora/client/cli/test_task.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_task.py b/src/test/python/apache/aurora/client/cli/test_task.py
index 5432a3d..d2233b6 100644
--- a/src/test/python/apache/aurora/client/cli/test_task.py
+++ b/src/test/python/apache/aurora/client/cli/test_task.py
@@ -147,8 +147,7 @@ class TestSshCommand(AuroraClientCommandTest):
jobKeys=[JobKey(role='bozo', environment='test', name='hello')],
instanceIds=set([1]),
statuses=set([ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.RESTARTING,
- ScheduleStatus.PREEMPTING, ScheduleStatus.DRAINING
- ])))
+ ScheduleStatus.PREEMPTING, ScheduleStatus.DRAINING])))
mock_subprocess.assert_called_with(['ssh', '-t', '-v', 'bozo@slavehost',
'cd /slaveroot/slaves/*/frameworks/*/executors/thermos-1287391823/runs/'
'slaverun/sandbox;ls'])