Skip to content

Commit

Permalink
Deprecate exception.message usages
Browse files Browse the repository at this point in the history
In fb30d2d

     if isinstance(cause, UnreadableArtifact):
       return (target_address, cause.err.message)

However, `exception.message` isn't always a string, it's `args[0]` whatever is passed as first argument [1],
maybe string, maybe another wrapped exception. This results a later error when called by json.dump [2]

Further, `exception.message` is deprecated in python 3 [1], this PR drops such usages

[1] https://www.python.org/dev/peps/pep-0352
[2] bug stacktrace

  File "/Users/peiyu/.pex/install/pantsbuild.pants-0.0.0_fb30d2db66b95d2bb602cbaae60037c18d2710b6-py2-none-any.whl.5632d5cbfcdd1b48dcdcbd328644dfab1f356c58/pantsbuild.pants-0.0.0_fb30d2db66b95d2bb602cbaae60037c18d2710b6-py2-none-any.whl/pants/goal/run_tracker.py", line 263, in store_stats
    safe_file_dump(stats_file, json.dumps(stats))
...
Exception message: ConnectionError(ProtocolError('Connection aborted.', gaierror(8, 'nodename nor servnam

Testing Done:
https://travis-ci.org/pantsbuild/pants/builds/94495459

Bugs closed: 2656

Reviewed at https://rbcommons.com/s/twitter/r/3201/
  • Loading branch information
peiyuwang authored and Peiyu Wang committed Dec 3, 2015
1 parent fb30d2d commit 660acf8
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/base/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def get_buildroot():
try:
return BuildRoot().path
except BuildRoot.NotFoundError as e:
print(e.message, file=sys.stderr)
print(str(e), file=sys.stderr)
sys.exit(1)


Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/goal/artifact_cache_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def format_vts(tgt, cause):
"""Format into (target, cause) tuple."""
target_address = tgt.address.reference()
if isinstance(cause, UnreadableArtifact):
return (target_address, cause.err.message)
return (target_address, str(cause.err))
elif cause == False:
return (target_address, 'uncached')
else:
Expand Down
2 changes: 1 addition & 1 deletion tests/python/pants_test/authentication/test_netrc_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_netrc_parse_error(self, MockOsPath):
with self.netrc('machine test') as netrc:
with self.assertRaises(netrc.NetrcError) as exc:
netrc._ensure_loaded()
assert re.search(r'Problem parsing', exc.exception.message)
assert re.search(r'Problem parsing', str(exc.exception))

def test_netrc_no_usable_blocks(self, MockOsPath):
with self.netrc('') as netrc:
Expand Down
15 changes: 9 additions & 6 deletions tests/python/pants_test/goal/test_artifact_cache_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import requests

from pants.cache.artifact import ArtifactError
from pants.cache.artifact_cache import UnreadableArtifact
from pants.cache.artifact_cache import NonfatalArtifactCacheError, UnreadableArtifact
from pants.goal.artifact_cache_stats import ArtifactCacheStats
from pants.util.contextutil import temporary_dir
from pants_test.base_test import BaseTest
Expand All @@ -21,7 +21,10 @@ class ArtifactCacheStatsTest(BaseTest):
TEST_CACHE_NAME_1 = 'ZincCompile'
TEST_CACHE_NAME_2 = 'Checkstyle_test_checkstyle'
TEST_LOCAL_ERROR = UnreadableArtifact('foo', ArtifactError('CRC check failed'))
TEST_REMOTE_ERROR = UnreadableArtifact('bar', requests.exceptions.ConnectionError('Read time out'))
TEST_REMOTE_ERROR = UnreadableArtifact(
'bar',
NonfatalArtifactCacheError(requests.exceptions.ConnectionError('Read time out'))
)
TEST_SPEC_A = 'src/scala/a'
TEST_SPEC_B = 'src/scala/b'
TEST_SPEC_C = 'src/java/c'
Expand All @@ -40,24 +43,24 @@ def test_add_hits(self):
'num_hits': 0,
'num_misses': 1,
'hits': [],
'misses': [(self.TEST_SPEC_A, self.TEST_LOCAL_ERROR.err.message)]
'misses': [(self.TEST_SPEC_A, str(self.TEST_LOCAL_ERROR.err))]
},
{
'cache_name': self.TEST_CACHE_NAME_1,
'num_hits': 1,
'num_misses': 1,
'hits': [(self.TEST_SPEC_B, '')],
'misses': [(self.TEST_SPEC_C, self.TEST_REMOTE_ERROR.err.message)]
'misses': [(self.TEST_SPEC_C, str(self.TEST_REMOTE_ERROR.err))]
},
]

expected_hit_or_miss_files = {
'{}.misses'.format(self.TEST_CACHE_NAME_2):
'{} {}\n'.format(self.TEST_SPEC_A, self.TEST_LOCAL_ERROR.err.message),
'{} {}\n'.format(self.TEST_SPEC_A, str(self.TEST_LOCAL_ERROR.err)),
'{}.hits'.format(self.TEST_CACHE_NAME_1):
'{}\n'.format(self.TEST_SPEC_B),
'{}.misses'.format(self.TEST_CACHE_NAME_1):
'{} {}\n'.format(self.TEST_SPEC_C, self.TEST_REMOTE_ERROR.err.message),
'{} {}\n'.format(self.TEST_SPEC_C, str(self.TEST_REMOTE_ERROR.err)),
}

with self.mock_artifact_cache_stats(expected_stats,
Expand Down

0 comments on commit 660acf8

Please sign in to comment.