From d159d689c5111c0bf53f3f441b8fdf766c9e763d Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 5 Jun 2019 17:03:13 -0700 Subject: [PATCH 1/4] Fix py-thrift-namespace-clash-check logging issue when using --no-strict --- .../codegen/thrift/python/py_thrift_namespace_clash_check.py | 4 ++-- src/python/pants/reporting/report.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py b/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py index 0dbd9f8c3f1..dc28b21f224 100644 --- a/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py +++ b/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py @@ -101,7 +101,7 @@ def _extract_all_python_namespaces(self, thrift_file_sources_by_target): if self.get_options().strict: raise error else: - self.context.log.warn(error) + self.context.log.warn(str(error)) return py_namespaces_by_target class ClashingNamespaceError(TaskError): pass @@ -136,7 +136,7 @@ def _determine_clashing_namespaces(self, py_namespaces_by_target): if self.get_options().strict: raise error else: - self.context.log.warn(error) + self.context.log.warn(str(error)) return namespaces_by_files def execute(self): diff --git a/src/python/pants/reporting/report.py b/src/python/pants/reporting/report.py index b981d444931..0ec4c5ee517 100644 --- a/src/python/pants/reporting/report.py +++ b/src/python/pants/reporting/report.py @@ -100,7 +100,8 @@ def start_workunit(self, workunit): def log(self, workunit, level, *msg_elements): """Log a message. - Each element of msg_elements is either a message string or a (message, detail) pair. + Each element of msg_elements is either a message or a (message, detail) pair, i.e. of type + Union[str, Tuple[str, str]]. """ with self._lock: for reporter in self._reporters.values(): From 380c356120ef855f7874235e12e7d99c21fe708d Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 5 Jun 2019 17:09:07 -0700 Subject: [PATCH 2/4] Add assertion to avoid this from happening again. --- src/python/pants/reporting/report.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/python/pants/reporting/report.py b/src/python/pants/reporting/report.py index 0ec4c5ee517..06df31c6b6c 100644 --- a/src/python/pants/reporting/report.py +++ b/src/python/pants/reporting/report.py @@ -103,6 +103,10 @@ def log(self, workunit, level, *msg_elements): Each element of msg_elements is either a message or a (message, detail) pair, i.e. of type Union[str, Tuple[str, str]]. """ + # TODO(6742): Once we have enough MyPy coverage, we can rely on MyPy to catch any issues for us, + # rather than this runtime check. + assert all(isinstance(element, (str, tuple)) for element in msg_elements), \ + "At least one logged message element is not of type Union[str, Tuple[str, str]]:\n {}".format(msg_elements) with self._lock: for reporter in self._reporters.values(): reporter.handle_log(workunit, level, *msg_elements) From 07b81850c362436ec26411d4bf9404823bfd6949 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 5 Jun 2019 17:13:41 -0700 Subject: [PATCH 3/4] Add builtins imports --- .../codegen/thrift/python/py_thrift_namespace_clash_check.py | 2 +- src/python/pants/reporting/report.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py b/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py index dc28b21f224..9b5883fceeb 100644 --- a/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py +++ b/src/python/pants/backend/codegen/thrift/python/py_thrift_namespace_clash_check.py @@ -6,7 +6,7 @@ import os import re -from builtins import zip +from builtins import str, zip from pants.backend.codegen.thrift.python.python_thrift_library import PythonThriftLibrary from pants.base.exceptions import TaskError diff --git a/src/python/pants/reporting/report.py b/src/python/pants/reporting/report.py index 06df31c6b6c..5380b9dd109 100644 --- a/src/python/pants/reporting/report.py +++ b/src/python/pants/reporting/report.py @@ -6,7 +6,7 @@ import threading import time -from builtins import object +from builtins import object, str class ReportingError(Exception): From dd7022f4d17ced64c5c44dcc124de6a7e5b94c19 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Thu, 6 Jun 2019 01:09:58 -0700 Subject: [PATCH 4/4] Allow bytes so that Python 2 works properly --- src/python/pants/reporting/report.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/python/pants/reporting/report.py b/src/python/pants/reporting/report.py index 5380b9dd109..dfc9811740a 100644 --- a/src/python/pants/reporting/report.py +++ b/src/python/pants/reporting/report.py @@ -6,7 +6,7 @@ import threading import time -from builtins import object, str +from builtins import bytes, object, str class ReportingError(Exception): @@ -101,12 +101,14 @@ def log(self, workunit, level, *msg_elements): """Log a message. Each element of msg_elements is either a message or a (message, detail) pair, i.e. of type - Union[str, Tuple[str, str]]. + Union[str, bytes, Tuple[str, str]]. """ # TODO(6742): Once we have enough MyPy coverage, we can rely on MyPy to catch any issues for us, # rather than this runtime check. - assert all(isinstance(element, (str, tuple)) for element in msg_elements), \ - "At least one logged message element is not of type Union[str, Tuple[str, str]]:\n {}".format(msg_elements) + # TODO(6071): No longer allow bytes once Py2 is removed. + assert all(isinstance(element, (str, bytes, tuple)) for element in msg_elements), \ + "At least one logged message element is not of type " \ + "Union[str, bytes, Tuple[str, str]]:\n {}".format(msg_elements) with self._lock: for reporter in self._reporters.values(): reporter.handle_log(workunit, level, *msg_elements)