Skip to content

Commit

Permalink
Fix py-thrift-namespace-clash-check type issue when logging with `-…
Browse files Browse the repository at this point in the history
…-no-strict` mode (#7864)

### Problem
When running `py-thrift-namespace-clash-check` in Twitter's sandbox with `--no-strict` mode, Pants crashed with the exception:

```
zip_longest argument #1 must support iteration
```

This is because we were trying to log a `NamespaceExtractionError`, and `log()` only understands `Union[str, Tuple[str, str]]`.

### Solution
Fix the issue by using `str()` to get the underlying message.

Also add an assertion to make sure that we don't introduce this issue again. Once we have MyPy set up, we can remove this assertion.
  • Loading branch information
Eric-Arellano authored and stuhood committed Jun 6, 2019
1 parent bcd5599 commit d8be628
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
11 changes: 9 additions & 2 deletions src/python/pants/reporting/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import threading
import time
from builtins import object
from builtins import bytes, object, str


class ReportingError(Exception):
Expand Down Expand Up @@ -100,8 +100,15 @@ 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, 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.
# 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)
Expand Down

0 comments on commit d8be628

Please sign in to comment.