Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate JSON stats recording in RunTracker #7446

Merged
merged 2 commits into from Apr 1, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 9 additions & 25 deletions src/python/pants/goal/run_tracker.py
Expand Up @@ -13,14 +13,12 @@
import threading
import time
import uuid
from builtins import open
from contextlib import contextmanager

import requests
from future.utils import PY2, PY3
from future.utils import PY3

from pants.auth.cookies import Cookies
from pants.base.build_environment import get_pants_cachedir
from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE
from pants.base.run_info import RunInfo
from pants.base.worker_pool import SubprocPool, WorkerPool
Expand Down Expand Up @@ -394,21 +392,14 @@ def _json_dump_options(cls, stats):

@classmethod
def write_stats_to_json(cls, file_name, stats):
"""Write stats to a local json file.

:return: True if successfully written, False otherwise.
"""
"""Write stats to a local json file."""
params = cls._json_dump_options(stats)
if PY2:
params = params.decode('utf-8')
mode = 'w' if PY3 else 'wb'
try:
with open(file_name, 'w') as f:
f.write(params)
except Exception as e: # Broad catch - we don't want to fail in stats related failure.
safe_file_dump(file_name, params, mode=mode)
except Exception as e: # Broad catch - we don't want to fail in stats related failure.
print('WARNING: Failed to write stats to {} due to Error: {}'.format(file_name, e),
file=sys.stderr)
return False
return True

def store_stats(self):
"""Store stats about this run in local and optionally remote stats dbs."""
Expand All @@ -428,12 +419,10 @@ def store_stats(self):
'recorded_options': self._get_options_to_record(),
}

# Dump individual stat file.
# TODO(benjy): Do we really need these, once the statsdb is mature?
stats_file = os.path.join(get_pants_cachedir(), 'stats',
'{}.json'.format(self.run_info.get_info('id')))
mode = 'w' if PY3 else 'wb'
safe_file_dump(stats_file, self._json_dump_options(stats), mode=mode)
# Write stats to user-defined json file.
stats_json_file_name = self.get_options().stats_local_json_file
if stats_json_file_name:
self.write_stats_to_json(stats_json_file_name, stats)

# Add to local stats db.
StatsDBFactory.global_instance().get_db().insert_stats(stats)
Expand All @@ -444,11 +433,6 @@ def store_stats(self):
for stats_url, auth_provider in stats_upload_urls.items():
self.post_stats(stats_url, stats, timeout=timeout, auth_provider=auth_provider)

# Write stats to local json file.
stats_json_file_name = self.get_options().stats_local_json_file
if stats_json_file_name:
self.write_stats_to_json(stats_json_file_name, stats)

_log_levels = [Report.ERROR, Report.ERROR, Report.WARN, Report.INFO, Report.INFO]

def has_ended(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/python/pants_test/goal/test_run_tracker.py
Expand Up @@ -67,7 +67,7 @@ def test_write_stats_to_json_file(self):

# Execute & verify
with temporary_file_path() as file_name:
self.assertTrue(RunTracker.write_stats_to_json(file_name, stats))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the only place I could find where we used the return value from that method - since we're testing the file exists / matches output below that seemed unnecessary (and I've removed the boolean return values in the method itself).

RunTracker.write_stats_to_json(file_name, stats)
with open(file_name, 'r') as f:
result = json.load(f)
self.assertEqual(stats, result)
Expand Down