You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2021/10/24 13:42:08 UTC
[skywalking-python] branch master updated: Introduce another set of
flake8 extensions (#174)
This is an automated email from the ASF dual-hosted git repository.
kezhenxu94 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking-python.git
The following commit(s) were added to refs/heads/master by this push:
new 0af1e57 Introduce another set of flake8 extensions (#174)
0af1e57 is described below
commit 0af1e57beac3d9c92a49e310030153b281fde1cc
Author: Superskyyy <Su...@outlook.com>
AuthorDate: Sun Oct 24 09:42:03 2021 -0400
Introduce another set of flake8 extensions (#174)
---
.flake8 | 25 -----------
Makefile | 29 ++++++++++---
docs/en/contribution/CodingStyle.md | 19 +++++++--
requirements-style.txt | 22 ++++++++++
requirements.txt | 2 -
setup.cfg | 49 ++++++++++++++++++++++
skywalking/__init__.py | 2 +-
skywalking/log/formatter.py | 2 +-
skywalking/loggings.py | 2 +-
skywalking/plugins/__init__.py | 4 +-
skywalking/plugins/sw_http_server.py | 2 +-
skywalking/plugins/sw_kafka.py | 4 +-
skywalking/plugins/sw_pymongo.py | 6 +--
skywalking/plugins/sw_sanic.py | 4 +-
skywalking/profile/snapshot.py | 2 +-
skywalking/trace/context.py | 7 ++--
skywalking/utils/lang.py | 2 +-
sw_python/__main__.py | 2 +-
sw_python/{__main__.py => src/__init__.py} | 4 --
sw_python/{ => src}/sw_python.py | 0
tests/plugin/http/sw_http/services/consumer.py | 2 +-
tests/plugin/http/sw_http/services/provider.py | 2 +-
.../plugin/http/sw_http_wsgi/services/consumer.py | 2 +-
tests/plugin/http/sw_requests/services/consumer.py | 2 +-
tests/plugin/http/sw_requests/services/provider.py | 2 +-
tools/check-license-header.py | 2 +-
tools/codegen.py | 2 +-
tools/doc/plugin_doc_gen.py | 2 +-
28 files changed, 138 insertions(+), 67 deletions(-)
diff --git a/.flake8 b/.flake8
deleted file mode 100644
index ff8854f..0000000
--- a/.flake8
+++ /dev/null
@@ -1,25 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you 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.
-
-[flake8]
-# careful with E303, which may cause too many blank lines in pycharm formatted code
-ignore = E501, E126, W503
-extend-ignore = FS003
-max-line-length = 120
-max-complexity = 20
-exclude = venv*,*egg_info
-count = True
-show-source = True
diff --git a/Makefile b/Makefile
index 8ee600b..71ab7a0 100644
--- a/Makefile
+++ b/Makefile
@@ -16,6 +16,13 @@
VERSION ?= latest
+# determine host platform
+VENV_DIR = venv
+VENV = $(VENV_DIR)/bin
+ifeq (win32,$(shell python3 -c "import sys; print(sys.platform)"))
+VENV=$(VENV_DIR)/Scripts
+endif
+
.PHONY: license
setup:
@@ -29,14 +36,26 @@ gen:
python3 -m grpc_tools.protoc --version || python3 -m pip install grpcio-tools
python3 tools/codegen.py
-# flake8 configurations should go to the file .flake8
+# flake8 configurations should go to the file setup.cfg
lint: clean
- flake8 --version || python3 -m pip install flake8 flake8-quotes flake8-use-fstring
+ python3 -m pip install -r requirements-style.txt
flake8 .
-dev-check:
- flake8 --version || python3 -m pip install flake8 flake8-quotes flake8-use-fstring
- flake8 .
+# used in development
+dev-setup:
+ $(VENV)/python -m pip install -r requirements-style.txt
+
+dev-check: dev-setup
+ $(VENV)/flake8 .
+
+# fix problems described in CodingStyle.md - verify outcome with extra care
+dev-fix: dev-setup
+ $(VENV)/isort .
+ $(VENV)/unify -r --in-place .
+ $(VENV)/flynt -tc -v .
+
+doc-gen:
+ $(VENV)/python tools/doc/plugin_doc_gen.py
license: clean
python3 tools/check-license-header.py skywalking tests tools
diff --git a/docs/en/contribution/CodingStyle.md b/docs/en/contribution/CodingStyle.md
index 076fc5c..37e488f 100644
--- a/docs/en/contribution/CodingStyle.md
+++ b/docs/en/contribution/CodingStyle.md
@@ -6,7 +6,7 @@ Since Python 3.5 is end of life, we fully utilize the clarity and performance bo
Please do not use other styles - `+`, `%` or `.format` unless f-string is absolutely unfeasible in the context, or
it is a logger message, which is [optimized](https://docs.python.org/3/howto/logging.html#optimization) for the `%` style
-Run [flynt](https://github.com/ikamensh/flynt) to convert other formats to f-string, pay **extra care** to possible corner
+Run `make dev-fix` to invoke [flynt](https://github.com/ikamensh/flynt) to convert other formats to f-string, pay **extra care** to possible corner
cases leading to a semantically different conversion.
### Quotes
@@ -23,7 +23,7 @@ foo = f"I'm a string"
bar = f"This repo is called 'skywalking-python'"
```
-Run [unify](https://github.com/myint/unify) `unify -r your_target --in-place` to fix your quotes if flake8 complaints about it.
+Run `make dev-fix` to invoke [unify](https://github.com/myint/unify) to deal with your quotes if flake8 complaints about it.
## Debug messages
Please import the `logger_debug_enabled` variable and wrap your debug messages with a check.
@@ -33,4 +33,17 @@ This should be done for all performance critical components.
```python
if logger_debug_enabled:
logger.debug('Message - %s', some_func())
-```
\ No newline at end of file
+```
+
+# Imports
+Please make sure the imports are placed in a good order, or flake8-isort will notify you of the violations.
+
+Run `make dev-fix` to automatically fix the sorting problem.
+
+# Naming
+In PEP8 convention, we are required to use snake_case as the accepted style.
+
+However, there are special cases. For example, you are overriding/monkey-patching a method which happens to use the old style camelCase naming,
+then it is acceptable to have the original naming convention to preserve context.
+
+Please mark the line with `# noqa` to avoid linting.
diff --git a/requirements-style.txt b/requirements-style.txt
new file mode 100644
index 0000000..76bc242
--- /dev/null
+++ b/requirements-style.txt
@@ -0,0 +1,22 @@
+flake8 # Plain flake8
+
+## AUTO FIXERS
+#isort # import sorter
+unify # unify quotes
+flynt # fix f-strings
+
+## CODING STYLE
+flake8-quotes # checks for inconsistent single/double quotes
+flake8-use-fstring # enforce usage of f-strings
+pep8-naming # check pep8 naming convention
+Darglint # docstring description matches the definition
+flake8-eradicate # dead code checker (commented)
+#flake8-isort # isort checker | disabled temporarily
+flake8-docstrings # docstring checker
+
+## DESIGN CHECKER
+flake8-bugbear # bug checker
+flake8-comprehensions # misuse of list comprehension
+
+## REGRESSION CHECKER
+flake8-2020 # misuse of version_check in Py3.10 or future 4
\ No newline at end of file
diff --git a/requirements.txt b/requirements.txt
index 3c54cbf..daa7800 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,4 @@
pytest
-unify
-flynt
grpcio_tools==1.41.0
wrapt==1.13.2
requests==2.26.0
diff --git a/setup.cfg b/setup.cfg
new file mode 100644
index 0000000..0e12735
--- /dev/null
+++ b/setup.cfg
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+[flake8]
+
+# run with `--disable-noqa` to reevaluate skipped inspections
+# Do not use ignore option, it will override some plugin's default ignore
+extend-ignore =
+ E303, # Too many blank lines | Conflict with Pycharm
+ E126, # Continuation line over-indented for hanging indent | Conflict with Pycharm
+ E501, # Line too long | Customized by max-line-length
+ W503, # Line break occurred before a binary operator | Conflict with W504
+ # temporarily banned rules for various reasons, likely to re-enable in the future
+ E800, # Found commented out code | Temporarily disabled for legacy code
+ D, # docstring | To be enhanced, large number
+ DAR, # docstring structure | To be fixed
+
+
+max-line-length = 120
+max-complexity = 20
+exclude = venv*,*egg_info,skywalking/protocol
+count = True
+show-source = True
+statistics = True
+docstring-convention = google
+
+[bandit]
+exclude = tests,docs
+tests: B101
+
+[darglint]
+docstring_style = google
+
+[isort]
+line_length = 120
+multi_line_output = 11
diff --git a/skywalking/__init__.py b/skywalking/__init__.py
index 9e57f36..8398e52 100644
--- a/skywalking/__init__.py
+++ b/skywalking/__init__.py
@@ -78,6 +78,6 @@ LogItem = namedtuple('LogItem', 'key val')
class Log(object):
- def __init__(self, timestamp: time = time.time(), items: List[LogItem] = None):
+ def __init__(self, timestamp: time = time.time(), items: List[LogItem] = None): # noqa
self.timestamp = timestamp
self.items = items or []
diff --git a/skywalking/log/formatter.py b/skywalking/log/formatter.py
index bce71d0..eb0a74c 100644
--- a/skywalking/log/formatter.py
+++ b/skywalking/log/formatter.py
@@ -27,7 +27,7 @@ class SWFormatter(logging.Formatter):
logging.Formatter.__init__(self, fmt)
self.tb_limit = tb_limit
- def formatException(self, ei):
+ def formatException(self, ei): # noqa
sio = io.StringIO()
tb = ei[2]
traceback.print_exception(ei[0], ei[1], tb, self.tb_limit, sio)
diff --git a/skywalking/loggings.py b/skywalking/loggings.py
index a4ed05f..52d2e2b 100644
--- a/skywalking/loggings.py
+++ b/skywalking/loggings.py
@@ -20,7 +20,7 @@ import logging
from skywalking import config
-def getLogger(name=None):
+def getLogger(name=None): # noqa
logger = logging.getLogger(name)
ch = logging.StreamHandler()
formatter = logging.Formatter('%(name)s [%(threadName)s] [%(levelname)s] %(message)s')
diff --git a/skywalking/plugins/__init__.py b/skywalking/plugins/__init__.py
index 3da7a8d..d8298cf 100644
--- a/skywalking/plugins/__init__.py
+++ b/skywalking/plugins/__init__.py
@@ -36,7 +36,7 @@ def install():
disable_patterns = [re.compile(p.strip()) for p in disable_patterns.split(',') if p.strip()]
else:
disable_patterns = [re.compile(p.strip()) for p in disable_patterns if p.strip()]
- for importer, modname, ispkg in pkgutil.iter_modules(skywalking.plugins.__path__):
+ for importer, modname, _ispkg in pkgutil.iter_modules(skywalking.plugins.__path__):
if any(pattern.match(modname) for pattern in disable_patterns):
logger.info("plugin %s is disabled and thus won't be installed", modname)
continue
@@ -50,7 +50,7 @@ def install():
"won't be installed", modname)
continue
- if not hasattr(plugin, 'install') or inspect.ismethod(getattr(plugin, 'install')):
+ if not hasattr(plugin, 'install') or inspect.ismethod(plugin.install):
logger.warning("no `install` method in plugin %s, thus the plugin won't be installed", modname)
continue
diff --git a/skywalking/plugins/sw_http_server.py b/skywalking/plugins/sw_http_server.py
index 4783ee8..e0e116b 100644
--- a/skywalking/plugins/sw_http_server.py
+++ b/skywalking/plugins/sw_http_server.py
@@ -102,7 +102,7 @@ def wrap_werkzeug_request_handler(handler):
return _send_response(self, code, *args, **kwargs)
- WSGIRequestHandler = handler.__class__
+ WSGIRequestHandler = handler.__class__ # noqa
if not getattr(WSGIRequestHandler, '_sw_wrapped', False):
_send_response = WSGIRequestHandler.send_response
diff --git a/skywalking/plugins/sw_kafka.py b/skywalking/plugins/sw_kafka.py
index 1995ee6..79e6646 100644
--- a/skywalking/plugins/sw_kafka.py
+++ b/skywalking/plugins/sw_kafka.py
@@ -52,8 +52,8 @@ def _sw__poll_once_func(__poll_once):
with context.new_entry_span(
op=f"Kafka/{topics}/Consumer/{this.config['group_id'] or ''}") as span:
- for consumerRecords in res.values():
- for record in consumerRecords:
+ for consumer_records in res.values():
+ for record in consumer_records:
carrier = Carrier()
for item in carrier:
for header in record.headers:
diff --git a/skywalking/plugins/sw_pymongo.py b/skywalking/plugins/sw_pymongo.py
index fc15302..9c8badc 100644
--- a/skywalking/plugins/sw_pymongo.py
+++ b/skywalking/plugins/sw_pymongo.py
@@ -48,7 +48,7 @@ def install():
inject_socket_info(SocketInfo)
-def inject_socket_info(SocketInfo):
+def inject_socket_info(SocketInfo): # noqa
_command = SocketInfo.command
def _sw_command(this: SocketInfo, dbname, spec, *args, **kwargs):
@@ -100,7 +100,7 @@ def _get_filter(request_type, spec):
return f'{request_type} {str(spec)}'
-def inject_bulk_write(_Bulk, bulk_op_map):
+def inject_bulk_write(_Bulk, bulk_op_map): # noqa
_execute = _Bulk.execute
def _sw_execute(this: _Bulk, *args, **kwargs):
@@ -133,7 +133,7 @@ def inject_bulk_write(_Bulk, bulk_op_map):
_Bulk.execute = _sw_execute
-def inject_cursor(Cursor):
+def inject_cursor(Cursor): # noqa
__send_message = Cursor._Cursor__send_message
def _sw_send_message(this: Cursor, operation):
diff --git a/skywalking/plugins/sw_sanic.py b/skywalking/plugins/sw_sanic.py
index 271d84e..e374898 100644
--- a/skywalking/plugins/sw_sanic.py
+++ b/skywalking/plugins/sw_sanic.py
@@ -46,7 +46,7 @@ def install():
_format_http1_response = response.format_http1_response
_handle_request = Sanic.handle_request
- _handlers_ErrorHandler_response = handlers.ErrorHandler.response
+ _handlers_ErrorHandler_response = handlers.ErrorHandler.response # noqa
def _sw_format_http1_response(status: int, headers, body=b''):
if status is not None:
@@ -58,7 +58,7 @@ def install():
return _format_http1_response(status, headers, body)
- def _sw_handlers_ErrorHandler_response(self: handlers.ErrorHandler, req, e):
+ def _sw_handlers_ErrorHandler_response(self: handlers.ErrorHandler, req, e): # noqa
if e is not None:
entry_span = get_context().active_span()
if entry_span is not None and type(entry_span) is not NoopSpan:
diff --git a/skywalking/profile/snapshot.py b/skywalking/profile/snapshot.py
index b3a8212..c087bdf 100644
--- a/skywalking/profile/snapshot.py
+++ b/skywalking/profile/snapshot.py
@@ -30,7 +30,7 @@ class TracingThreadSnapshot:
self.stack_list = stack_list
def transform(self) -> ThreadSnapshot:
- code_sigs = [code_sign for code_sign in self.stack_list]
+ code_sigs = list(self.stack_list)
stack = ThreadStack(codeSignatures=code_sigs)
snapshot = ThreadSnapshot(
diff --git a/skywalking/trace/context.py b/skywalking/trace/context.py
index 8846207..ec359d9 100644
--- a/skywalking/trace/context.py
+++ b/skywalking/trace/context.py
@@ -15,9 +15,10 @@
# limitations under the License.
#
-from skywalking.profile.profile_status import ProfileStatusReference
from skywalking import Component, agent, config
+from skywalking import profile
from skywalking.agent import isfull
+from skywalking.profile.profile_status import ProfileStatusReference
from skywalking.trace import ID
from skywalking.trace.carrier import Carrier
from skywalking.trace.segment import Segment, SegmentRef
@@ -25,8 +26,6 @@ from skywalking.trace.snapshot import Snapshot
from skywalking.trace.span import Span, Kind, NoopSpan, EntrySpan, ExitSpan
from skywalking.utils.counter import Counter
from skywalking.utils.time import current_milli_time
-from skywalking import profile
-
try: # attempt to use async-local instead of thread-local context and spans
import contextvars
@@ -87,7 +86,7 @@ class SpanContext(object):
return None
- def new_span(self, parent: Span, SpanType: type, **kwargs) -> Span:
+ def new_span(self, parent: Span, SpanType: type, **kwargs) -> Span: # noqa
finished = parent and not parent._depth
context = SpanContext() if finished else self
span = SpanType(context=context,
diff --git a/skywalking/utils/lang.py b/skywalking/utils/lang.py
index a60ba0a..46b1982 100644
--- a/skywalking/utils/lang.py
+++ b/skywalking/utils/lang.py
@@ -19,7 +19,7 @@ import base64
def tostring(cls):
- def __str__(self):
+ def __str__(self): # noqa
return f"{type(self).__name__}@{id(self)}[{', '.join(f'{k}={str(v)}' for (k, v) in vars(self).items())}]"
cls.__str__ = __str__
return cls
diff --git a/sw_python/__main__.py b/sw_python/__main__.py
index 5c978b1..adc5e45 100644
--- a/sw_python/__main__.py
+++ b/sw_python/__main__.py
@@ -15,6 +15,6 @@
# limitations under the License.
#
-from .sw_python import start
+from sw_python.src.sw_python import start
start()
diff --git a/sw_python/__main__.py b/sw_python/src/__init__.py
similarity index 95%
copy from sw_python/__main__.py
copy to sw_python/src/__init__.py
index 5c978b1..b1312a0 100644
--- a/sw_python/__main__.py
+++ b/sw_python/src/__init__.py
@@ -14,7 +14,3 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-
-from .sw_python import start
-
-start()
diff --git a/sw_python/sw_python.py b/sw_python/src/sw_python.py
similarity index 100%
rename from sw_python/sw_python.py
rename to sw_python/src/sw_python.py
diff --git a/tests/plugin/http/sw_http/services/consumer.py b/tests/plugin/http/sw_http/services/consumer.py
index c1ad889..bd9e769 100644
--- a/tests/plugin/http/sw_http/services/consumer.py
+++ b/tests/plugin/http/sw_http/services/consumer.py
@@ -24,7 +24,7 @@ if __name__ == '__main__':
from http.server import BaseHTTPRequestHandler
class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
- def do_POST(self):
+ def do_POST(self): # noqa
self.send_response(200)
self.send_header('Content-Type', 'application/json; charset=utf-8')
self.end_headers()
diff --git a/tests/plugin/http/sw_http/services/provider.py b/tests/plugin/http/sw_http/services/provider.py
index 4939cb8..d57d7e0 100644
--- a/tests/plugin/http/sw_http/services/provider.py
+++ b/tests/plugin/http/sw_http/services/provider.py
@@ -24,7 +24,7 @@ if __name__ == '__main__':
class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
- def do_POST(self):
+ def do_POST(self): # noqa
time.sleep(0.5)
self.send_response(200)
self.send_header('Content-Type', 'application/json')
diff --git a/tests/plugin/http/sw_http_wsgi/services/consumer.py b/tests/plugin/http/sw_http_wsgi/services/consumer.py
index 7881177..41e5e4f 100644
--- a/tests/plugin/http/sw_http_wsgi/services/consumer.py
+++ b/tests/plugin/http/sw_http_wsgi/services/consumer.py
@@ -23,7 +23,7 @@ if __name__ == '__main__':
from http.server import BaseHTTPRequestHandler
class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
- def do_POST(self):
+ def do_POST(self): # noqa
self.send_response(200)
self.send_header('Content-Type', 'application/json; charset=utf-8')
self.end_headers()
diff --git a/tests/plugin/http/sw_requests/services/consumer.py b/tests/plugin/http/sw_requests/services/consumer.py
index e725396..db0d10e 100644
--- a/tests/plugin/http/sw_requests/services/consumer.py
+++ b/tests/plugin/http/sw_requests/services/consumer.py
@@ -23,7 +23,7 @@ if __name__ == '__main__':
from http.server import BaseHTTPRequestHandler
class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
- def do_POST(self):
+ def do_POST(self): # noqa
self.send_response(200)
self.send_header('Content-Type', 'application/json; charset=utf-8')
self.end_headers()
diff --git a/tests/plugin/http/sw_requests/services/provider.py b/tests/plugin/http/sw_requests/services/provider.py
index 4939cb8..d57d7e0 100644
--- a/tests/plugin/http/sw_requests/services/provider.py
+++ b/tests/plugin/http/sw_requests/services/provider.py
@@ -24,7 +24,7 @@ if __name__ == '__main__':
class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
- def do_POST(self):
+ def do_POST(self): # noqa
time.sleep(0.5)
self.send_response(200)
self.send_header('Content-Type', 'application/json')
diff --git a/tools/check-license-header.py b/tools/check-license-header.py
index ce35ee6..9e71b10 100755
--- a/tools/check-license-header.py
+++ b/tools/check-license-header.py
@@ -48,7 +48,7 @@ comment_leading_chars = ('#', '::')
def walk_through_dir(d) -> bool:
checked = True
- for root, sub_dirs, files in os.walk(d):
+ for root, _sub_dirs, files in os.walk(d):
for filename in files:
file_path = os.path.join(root, filename)
with open(file_path, 'r') as f:
diff --git a/tools/codegen.py b/tools/codegen.py
index a2d7a81..d757590 100644
--- a/tools/codegen.py
+++ b/tools/codegen.py
@@ -38,7 +38,7 @@ def codegen():
f'--proto_path={src_dir}',
f'--python_out={dest_dir}',
f'--grpc_python_out={dest_dir}'
- ] + [proto for proto in glob.iglob(f'{src_dir}/**/*.proto')])
+ ] + list(glob.iglob(f'{src_dir}/**/*.proto')))
for py_file in glob.iglob(os.path.join(dest_dir, '**/*.py')):
touch(os.path.join(os.path.dirname(py_file), '__init__.py'))
diff --git a/tools/doc/plugin_doc_gen.py b/tools/doc/plugin_doc_gen.py
index bdc6d73..58f724e 100644
--- a/tools/doc/plugin_doc_gen.py
+++ b/tools/doc/plugin_doc_gen.py
@@ -50,7 +50,7 @@ def generate_plugin_doc():
"""
table_entries = []
note_entries = []
- for importer, modname, ispkg in pkgutil.iter_modules(plugins_path):
+ for importer, modname, _ispkg in pkgutil.iter_modules(plugins_path):
plugin = importer.find_module(modname).load_module(modname)
try: