Skip to content

Add flake8 and isort lint checks #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .isort.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[settings]
from_first=True
17 changes: 10 additions & 7 deletions opentelemetry-api/src/opentelemetry/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def my_factory_for_t(api_type: typing.Type[T]) -> typing.Optional[T]:
means that the Python interpreter was invoked with the ``-E`` or ``-I`` flag).
"""

from typing import Type, TypeVar, Optional, Callable
from typing import Callable, Optional, Type, TypeVar
import importlib
import os
import sys
Expand All @@ -78,10 +78,11 @@ def my_factory_for_t(api_type: typing.Type[T]) -> typing.Optional[T]:
# annotate setters, were it not for https://github.com/python/mypy/issues/7092
# Once that bug is resolved, setters should use this instead of duplicating the
# code.
#ImplementationFactory = Callable[[Type[_T]], Optional[_T]]
# ImplementationFactory = Callable[[Type[_T]], Optional[_T]]

_DEFAULT_FACTORY: Optional[_UntrustedImplFactory[object]] = None


def _try_load_impl_from_modname(
implementation_modname: str, api_type: Type[_T]) -> Optional[_T]:
try:
Expand All @@ -92,6 +93,7 @@ def _try_load_impl_from_modname(

return _try_load_impl_from_mod(implementation_mod, api_type)


def _try_load_impl_from_mod(
implementation_mod: object, api_type: Type[_T]) -> Optional[_T]:

Expand All @@ -109,6 +111,7 @@ def _try_load_impl_from_mod(

return _try_load_impl_from_callback(implementation_fn, api_type)


def _try_load_impl_from_callback(
implementation_fn: _UntrustedImplFactory[_T],
api_type: Type[_T]
Expand Down Expand Up @@ -149,11 +152,10 @@ def _try_load_configured_impl(
return _try_load_impl_from_callback(_DEFAULT_FACTORY, api_type)
return None


# Public to other opentelemetry-api modules
def _load_impl(
api_type: Type[_T],
factory: Optional[Callable[[Type[_T]], Optional[_T]]]
) -> _T:
def _load_impl(api_type: Type[_T],
factory: Optional[Callable[[Type[_T]], Optional[_T]]]) -> _T:
"""Tries to load a configured implementation, if unsuccessful, returns a
fast no-op implemenation that is always available.
"""
Expand All @@ -163,9 +165,10 @@ def _load_impl(
return api_type()
return result


def set_preferred_default_implementation(
implementation_factory: _UntrustedImplFactory[_T]) -> None:
"""Sets a factory function that may be called for any implementation
object. See the :ref:`module docs <loader-factory>` for more details."""
global _DEFAULT_FACTORY #pylint:disable=global-statement
global _DEFAULT_FACTORY # pylint:disable=global-statement
_DEFAULT_FACTORY = implementation_factory
1 change: 1 addition & 0 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@

from contextlib import contextmanager
import typing

from opentelemetry import loader


Expand Down
16 changes: 11 additions & 5 deletions opentelemetry-api/tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,28 @@
# limitations under the License.

from importlib import reload
import os
import sys
import unittest
import os

from opentelemetry import loader
from opentelemetry import trace

DUMMY_TRACER = None


class DummyTracer(trace.Tracer):
pass


def get_opentelemetry_implementation(type_):
global DUMMY_TRACER #pylint:disable=global-statement
global DUMMY_TRACER # pylint:disable=global-statement
assert type_ is trace.Tracer
DUMMY_TRACER = DummyTracer()
return DUMMY_TRACER

#pylint:disable=redefined-outer-name,protected-access,unidiomatic-typecheck

# pylint:disable=redefined-outer-name,protected-access,unidiomatic-typecheck

class TestLoader(unittest.TestCase):

Expand All @@ -43,7 +46,6 @@ def setUp(self):
# class after reloading `trace`.
reload(sys.modules[__name__])


def test_get_default(self):
tracer = trace.tracer()
self.assertIs(type(tracer), trace.Tracer)
Expand All @@ -60,8 +62,10 @@ def do_test_preferred_impl(self, setter):
setter(get_opentelemetry_implementation)
tracer = trace.tracer()
self.assertIs(tracer, DUMMY_TRACER)

def test_preferred_impl_with_tracer(self):
self.do_test_preferred_impl(trace.set_preferred_tracer_implementation)

def test_preferred_impl_with_default(self):
self.do_test_preferred_impl(
loader.set_preferred_default_implementation)
Expand All @@ -75,7 +79,7 @@ def test_try_set_again(self):
self.assertIn('already loaded', str(einfo.exception))

def do_test_get_envvar(self, envvar_suffix):
global DUMMY_TRACER #pylint:disable=global-statement
global DUMMY_TRACER # pylint:disable=global-statement

# Test is not runnable with this!
self.assertFalse(sys.flags.ignore_environment)
Expand All @@ -89,7 +93,9 @@ def do_test_get_envvar(self, envvar_suffix):
DUMMY_TRACER = None
del os.environ[envname]
self.assertIs(type(tracer), DummyTracer)

def test_get_envvar_tracer(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to capture the missing blank line here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually these were intentional as the three functions are a "translation" of a single @pytest.mark.parametrize function. But you are right, in general, having a rule to enforce at least a single blank line between functions would be good.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tox -e lint does catch these, travis just isn't running lint.

return self.do_test_get_envvar('TRACER')

def test_get_envvar_default(self):
return self.do_test_get_envvar('DEFAULT')
32 changes: 20 additions & 12 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
[tox]
skipsdist = True
envlist = py37-{lint,mypy,test}, docs
envlist = py{34,35,36,37}-test, lint, py37-mypy, docs

[travis]
python =
3.7: py37, docs
3.7: py37, lint, docs

[testenv]
deps =
lint: pylint~=2.3.1
mypy: mypy~=0.711

setenv =
Expand All @@ -18,23 +17,32 @@ setenv =
changedir =
test: opentelemetry-api/tests


commands =
; Prefer putting everything in one pylint command to profit from duplication
; warnings.
py37-lint: pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ opentelemetry-api/setup.py
py37-mypy: mypy opentelemetry-api/src/opentelemetry/
; For test code, we don't want to enforce the full mypy strictness
py37-mypy: mypy opentelemetry-api/src/opentelemetry/
py37-mypy: mypy --config-file=mypy-relaxed.ini opentelemetry-api/tests/ opentelemetry-api/setup.py
test: python -m unittest discover

[testenv:lint]
deps =
pylint~=2.3
flake8~=3.7
isort~=4.3

commands =
; Prefer putting everything in one pylint command to profit from duplication
; warnings.
pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/
flake8 opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/
isort --check-only --recursive opentelemetry-api/src

[testenv:docs]
deps =
sphinx~=2.1.2
sphinx-rtd-theme~=0.4.3
sphinx-autodoc-typehints~=1.6.0
sphinx~=2.1
sphinx-rtd-theme~=0.4
sphinx-autodoc-typehints~=1.6

changedir = docs

commands =
sphinx-build -W --keep-going -b html -T . _build/html
changedir = docs