Skip to content

Commit

Permalink
make deprecations respect warnings filters and add --ignore-pants-war…
Browse files Browse the repository at this point in the history
…nings option (#7496)

### Problem

#7493 solved its problem by converting an error to a warning, but it's not likely that a large monorepo is going to be able to solve that particular issue anytime soon. So while we would like to be able to deprecate the `--strict` option in that PR, we don't think it's reasonable to have a deprecation warning showing for the strict option immediately. But not every pants user is a large monorepo, so we would like to be able to do the right upstream thing (add a deprecation warning) while allowing downstream users to defer showing specific warnings in a structured way.

### Solution

- Add an `--ignore-pants-warnings` global bootstrap option, which calls [`warnings.filterwarnings()`](https://docs.python.org/2/library/warnings.html#warnings.filterwarnings) to filter warning messages by regex matching.
- Make `warn_or_error()` in `deprecated.py` use `warnings.warn_explicit()`, because it turns out that `warnings.showwarning()` doesn't respect the warning filters at all (oops!).

### Result

It's possible to filter out specific warnings with an option, and it's possible to do that to deprecation warnings now as well.
  • Loading branch information
cosmicexplorer committed Apr 12, 2019
1 parent 3f515b4 commit ef33b62
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 8 deletions.
39 changes: 33 additions & 6 deletions src/python/pants/base/deprecated.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import inspect import inspect
import sys import sys
import warnings import warnings
from contextlib import contextmanager
from functools import wraps from functools import wraps


import six import six
Expand Down Expand Up @@ -105,6 +106,28 @@ def _get_frame_info(stacklevel, context=1):
return frame_list[frame_stack_index] return frame_list[frame_stack_index]




@contextmanager
def _greater_warnings_context(context_lines_string):
"""Provide the `line` argument to warnings.showwarning().
warnings.warn_explicit() doesn't use the `line` argument to showwarning(), but we want to
make use of the warning filtering provided by warn_explicit(). This contextmanager overwrites the
showwarning() method to pipe in the desired amount of context lines when using warn_explicit().
"""
prev_showwarning = warnings.showwarning
def wrapped(message, category, filename, lineno, file=None, line=None):
return prev_showwarning(
message=message,
category=category,
filename=filename,
lineno=lineno,
file=file,
line=(line or context_lines_string))
warnings.showwarning = wrapped
yield
warnings.showwarning = prev_showwarning


# TODO: propagate `deprecation_start_version` to other methods in this file! # TODO: propagate `deprecation_start_version` to other methods in this file!
def warn_or_error(removal_version, deprecated_entity_description, hint=None, def warn_or_error(removal_version, deprecated_entity_description, hint=None,
deprecation_start_version=None, deprecation_start_version=None,
Expand All @@ -125,6 +148,8 @@ def warn_or_error(removal_version, deprecated_entity_description, hint=None,
:param int stacklevel: The stacklevel to pass to warnings.warn. :param int stacklevel: The stacklevel to pass to warnings.warn.
:param FrameInfo frame_info: If provided, use this frame info instead of getting one from :param FrameInfo frame_info: If provided, use this frame info instead of getting one from
`stacklevel`. `stacklevel`.
:param int context: The number of lines of source code surrounding the selected frame to display
in a warning message.
:param bool ensure_stderr: Whether use warnings.warn, or use warnings.showwarning to print :param bool ensure_stderr: Whether use warnings.warn, or use warnings.showwarning to print
directly to stderr. directly to stderr.
:raises DeprecationApplicationError: if the removal_version parameter is invalid. :raises DeprecationApplicationError: if the removal_version parameter is invalid.
Expand Down Expand Up @@ -160,16 +185,18 @@ def warn_or_error(removal_version, deprecated_entity_description, hint=None,


if removal_semver > PANTS_SEMVER: if removal_semver > PANTS_SEMVER:
if ensure_stderr: if ensure_stderr:
# No warning filters can stop us from printing this message directly to stderr.
warning_msg = warnings.formatwarning( warning_msg = warnings.formatwarning(
msg, DeprecationWarning, filename, line_number, line=context_lines) msg, DeprecationWarning, filename, line_number, line=context_lines)
print(warning_msg, file=sys.stderr) print(warning_msg, file=sys.stderr)
else: else:
warnings.showwarning( # This output is filtered by warning filters.
DeprecationWarning(msg) if PY2 else msg, with _greater_warnings_context(context_lines):
DeprecationWarning, warnings.warn_explicit(
filename, message=DeprecationWarning(msg) if PY2 else msg,
line_number, category=DeprecationWarning,
line=context_lines) filename=filename,
lineno=line_number)
return msg return msg
else: else:
raise CodeRemovedError(msg) raise CodeRemovedError(msg)
Expand Down
4 changes: 4 additions & 0 deletions src/python/pants/bin/pants_runner.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import logging import logging
import os import os
import sys import sys
import warnings
from builtins import object from builtins import object


from pants.base.exception_sink import ExceptionSink from pants.base.exception_sink import ExceptionSink
Expand Down Expand Up @@ -53,6 +54,9 @@ def run(self):
ExceptionSink.reset_should_print_backtrace_to_terminal(global_bootstrap_options.print_exception_stacktrace) ExceptionSink.reset_should_print_backtrace_to_terminal(global_bootstrap_options.print_exception_stacktrace)
ExceptionSink.reset_log_location(global_bootstrap_options.pants_workdir) ExceptionSink.reset_log_location(global_bootstrap_options.pants_workdir)


for message_regexp in global_bootstrap_options.ignore_pants_warnings:
warnings.filterwarnings(action='ignore', message=message_regexp)

if global_bootstrap_options.enable_pantsd: if global_bootstrap_options.enable_pantsd:
try: try:
return RemotePantsRunner(self._exiter, self._args, self._env, options_bootstrapper).run() return RemotePantsRunner(self._exiter, self._args, self._env, options_bootstrapper).run()
Expand Down
8 changes: 8 additions & 0 deletions src/python/pants/option/global_options.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import multiprocessing import multiprocessing
import os import os
import sys import sys
from builtins import str
from textwrap import dedent from textwrap import dedent


from pants.base.build_environment import (get_buildroot, get_default_pants_config_file, from pants.base.build_environment import (get_buildroot, get_default_pants_config_file,
Expand Down Expand Up @@ -115,6 +116,12 @@ def register_bootstrap_options(cls, register):
# after -l and -q in help output, which is conveniently contextual. # after -l and -q in help output, which is conveniently contextual.
register('--colors', type=bool, default=sys.stdout.isatty(), recursive=True, daemon=False, register('--colors', type=bool, default=sys.stdout.isatty(), recursive=True, daemon=False,
help='Set whether log messages are displayed in color.') help='Set whether log messages are displayed in color.')
# TODO(#7203): make a regexp option type!
register('--ignore-pants-warnings', type=list, member_type=str, default=[],
help='Regexps matching warning strings to ignore, e.g. '
'["DEPRECATED: scope some_scope will be removed"]. The regexps will be matched '
'from the start of the warning string, and will always be case-insensitive. '
'See the `warnings` module documentation for more background on these are used.')


register('--pants-version', advanced=True, default=pants_version(), register('--pants-version', advanced=True, default=pants_version(),
help='Use this pants version. Note Pants code only uses this to verify that you are ' help='Use this pants version. Note Pants code only uses this to verify that you are '
Expand Down Expand Up @@ -229,6 +236,7 @@ def register_bootstrap_options(cls, register):
help="Raise an exception if any targets declaring source files " help="Raise an exception if any targets declaring source files "
"fail to match any glob provided in the 'sources' argument.") "fail to match any glob provided in the 'sources' argument.")


# TODO(#7203): make a regexp option type!
register('--exclude-target-regexp', advanced=True, type=list, default=[], daemon=False, register('--exclude-target-regexp', advanced=True, type=list, default=[], daemon=False,
metavar='<regexp>', help='Exclude target roots that match these regexes.') metavar='<regexp>', help='Exclude target roots that match these regexes.')
register('--subproject-roots', type=list, advanced=True, default=[], register('--subproject-roots', type=list, advanced=True, default=[],
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@


from test_pants_plugin.pants_infra_tests import PantsInfraTests from test_pants_plugin.pants_infra_tests import PantsInfraTests
from test_pants_plugin.subsystems.pants_test_infra import PantsTestInfra from test_pants_plugin.subsystems.pants_test_infra import PantsTestInfra
from test_pants_plugin.tasks.deprecation_warning_task import DeprecationWarningTask
from test_pants_plugin.tasks.lifecycle_stub_task import LifecycleStubTask from test_pants_plugin.tasks.lifecycle_stub_task import LifecycleStubTask


from pants.build_graph.build_file_aliases import BuildFileAliases from pants.build_graph.build_file_aliases import BuildFileAliases
Expand All @@ -20,6 +21,7 @@ def build_file_aliases():
) )


def register_goals(): def register_goals():
task(name='deprecation-warning-task', action=DeprecationWarningTask).install()
task(name='lifecycle-stub-task', action=LifecycleStubTask).install('lifecycle-stub-goal') task(name='lifecycle-stub-task', action=LifecycleStubTask).install('lifecycle-stub-goal')




Expand Down
Original file line number Original file line Diff line number Diff line change
@@ -0,0 +1,18 @@
# coding=utf-8
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import sys

from pants.base.deprecated import warn_or_error
from pants.task.task import Task


class DeprecationWarningTask(Task):
"""Make a deprecation warning so that warning filters can be integration tested."""

def execute(self):
warn_or_error(removal_version='999.999.9.dev9',
deprecated_entity_description='This is a test warning!')
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -161,5 +161,6 @@ def test_setup_py_unregistered_pants_plugin(self):
'test_pants_plugin/subsystems/lifecycle_stubs.py', 'test_pants_plugin/subsystems/lifecycle_stubs.py',
'test_pants_plugin/tasks/', 'test_pants_plugin/tasks/',
'test_pants_plugin/tasks/__init__.py', 'test_pants_plugin/tasks/__init__.py',
'test_pants_plugin/tasks/deprecation_warning_task.py',
'test_pants_plugin/tasks/lifecycle_stub_task.py', 'test_pants_plugin/tasks/lifecycle_stub_task.py',
]) ])
4 changes: 4 additions & 0 deletions tests/python/pants_test/bin/BUILD
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@




python_tests( python_tests(
name='integration',
dependencies=[ dependencies=[
'src/python/pants/base:build_environment',
'src/python/pants/bin', 'src/python/pants/bin',
'src/python/pants/option',
'tests/python/pants_test:int-test', 'tests/python/pants_test:int-test',
'tests/python/pants_test/testutils:py2_compat',
], ],
tags = {'integration'}, tags = {'integration'},
) )
52 changes: 52 additions & 0 deletions tests/python/pants_test/bin/test_runner_integration.py
Original file line number Original file line Diff line number Diff line change
@@ -0,0 +1,52 @@
# coding=utf-8
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import os

from pants.base.build_environment import get_buildroot
from pants.option.scope import GLOBAL_SCOPE_CONFIG_SECTION
from pants_test.pants_run_integration_test import PantsRunIntegrationTest
from pants_test.testutils.py2_compat import assertRegex


class RunnerIntegrationTest(PantsRunIntegrationTest):
"""Test logic performed in PantsRunner."""

def _deprecation_warning_cmdline(self):
# Load the testprojects pants-plugins to get some testing tasks and subsystems.
testproject_backend_src_dir = os.path.join(
get_buildroot(), 'testprojects/pants-plugins/src/python')
testproject_backend_pkg_name = 'test_pants_plugin'
deprecation_warning_cmdline = [
'--no-enable-pantsd',
"--pythonpath=+['{}']".format(testproject_backend_src_dir),
"--backend-packages=+['{}']".format(testproject_backend_pkg_name),
# This task will always emit a DeprecationWarning.
'deprecation-warning-task',
]
return deprecation_warning_cmdline

def test_warning_filter(self):
cmdline = self._deprecation_warning_cmdline()

warning_run = self.run_pants(cmdline)
self.assert_success(warning_run)
assertRegex(
self,
warning_run.stderr_data,
'\\[WARN\\].*DeprecationWarning: DEPRECATED: This is a test warning!')

non_warning_run = self.run_pants(cmdline, config={
GLOBAL_SCOPE_CONFIG_SECTION: {
# NB: We do *not* include the exclamation point at the end, which tests that the regexps
# match from the beginning of the warning string, and don't require matching the entire
# string! We also lowercase the message to check that they are matched case-insensitively.
'ignore_pants_warnings': ['deprecated: this is a test warning']
},
})
self.assert_success(non_warning_run)
self.assertNotIn('DEPRECATED', non_warning_run.stderr_data)
self.assertNotIn('test warning', non_warning_run.stderr_data)
4 changes: 2 additions & 2 deletions tests/python/pants_test/option/test_options.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1363,7 +1363,7 @@ class DummyOptionable2(Optionable):
# Check that we got a warning, but not for the inherited option. # Check that we got a warning, but not for the inherited option.
single_warning_dummy1 = assert_single_element(w) single_warning_dummy1 = assert_single_element(w)
self.assertEqual(single_warning_dummy1.category, DeprecationWarning) self.assertEqual(single_warning_dummy1.category, DeprecationWarning)
self.assertNotIn('inherited', single_warning_dummy1.message) self.assertNotIn('inherited', str(single_warning_dummy1.message))


# Check values. # Check values.
# Deprecated scope takes precedence at equal rank. # Deprecated scope takes precedence at equal rank.
Expand All @@ -1378,7 +1378,7 @@ class DummyOptionable2(Optionable):
# Check that we got a warning. # Check that we got a warning.
single_warning_dummy2 = assert_single_element(w) single_warning_dummy2 = assert_single_element(w)
self.assertEqual(single_warning_dummy2.category, DeprecationWarning) self.assertEqual(single_warning_dummy2.category, DeprecationWarning)
self.assertNotIn('inherited', single_warning_dummy2.message) self.assertNotIn('inherited', str(single_warning_dummy2.message))


# Check values. # Check values.
self.assertEqual('uu', vals2.qux) self.assertEqual('uu', vals2.qux)
Expand Down

0 comments on commit ef33b62

Please sign in to comment.