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

Conversation

Projects
None yet
3 participants
@codealchemy
Copy link
Contributor

commented Mar 27, 2019

We currently write JSON two ways - one of which doesn't seem needed at all (writing to a cache that isn't accessed anywhere). This cleans that up a bit in preparation for adding the JsonReporter as an option for JSON output.

馃摑 One thing of note is the StatsDb schema is coupled to the current stats JSON.

c.execute("""INSERT INTO run_info VALUES (?, ?, ?, ?, ?, ?, ?, ?)""",
[ri['id'], int(float(ri['timestamp'])), ri['machine'], ri['user'],
ri['version'], ri['buildroot'], ri['outcome'], ri['cmd_line']])

If that view (<pants server url>/stats/) is not currently providing much value to users, there is quite a bit more that can be removed as a followup to this change.

codealchemy added some commits Mar 27, 2019

Drop writing a local json cache
It looks like this was added to survive a clean-all, but is not accessed. A User can determine if they'd like to write a JSON already for the build, so let's only write it then if configured.
@@ -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))

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 27, 2019

Author Contributor

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).

@benjyw

benjyw approved these changes Mar 27, 2019

@benjyw benjyw requested a review from stuhood Mar 27, 2019

@benjyw benjyw merged commit e9f9362 into pantsbuild:master Apr 1, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@codealchemy codealchemy deleted the codealchemy:update-run-tracker-0 branch Apr 1, 2019

benjyw added a commit that referenced this pull request Apr 2, 2019

Remove StatsDb / Timing stats views (#7475)
Followup to #7446.

The schema for StatsDB is coupled to the current output of stats used in the RunTracker - which complicates any attempts to move to a new stats schema (ie. the JsonReporter).

Given that the only usage of StatsDB was for the timing views from ./pants server, and those views are not heavily used (as far as could be determined in related Slack convo), this should be safe to remove.

jsirois added a commit that referenced this pull request Apr 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.