Closed
Bug 1419761
Opened 6 years ago
Closed 6 years ago
Rename "histogram_tools.py" to "parse_histograms.py"
Categories
(Toolkit :: Telemetry, enhancement, P3)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: Dexter, Assigned: leakey94, Mentored)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
Renaming this file [1] may look trivial, but there are some interesting dependencies: some upstream jobs (TMO aggregator?) might be fetching a copy of this file to do the parsing. As a consequence, when renaming this file, we should also point the dependent jobs to the new file name. [1] - https://searchfox.org/mozilla-central/rev/797c93d81fe446f78babf20894f0729f15f71ee6/toolkit/components/telemetry/histogram_tools.py
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Comment 1•6 years ago
|
||
Do you know if TMO (or any other project like the regression detector) fetches a copy of histogram_tools.py from mozilla-central?
Flags: needinfo?(chutten)
Comment 2•6 years ago
|
||
python_moztelemetry sorta does. It vendors a copy in using this script: https://github.com/mozilla/python_moztelemetry/blob/master/bin/update_histogram_tools The script will need updating, but as it appears to be run manually its failures would be noticed and rectified. The aggregator uses moztelemetry's version: https://github.com/mozilla/python_mozaggregator/search?utf8=%E2%9C%93&q=histogram&type= telemetry-batch-view doesn't use it. mozilla/moztelemetry seems to not use it, though I'm not 100% sure. ni?frank for any projects I'm missing.
Flags: needinfo?(chutten) → needinfo?(fbertsch)
Comment 3•6 years ago
|
||
Yup, the only dependency server-side should be python_moztelemetry; everywhere else should fetch it from there. We don't retrieve it in any Scala code, the histogram bucket generation is done separately in t-b-v.
Flags: needinfo?(fbertsch)
Reporter | ||
Comment 4•6 years ago
|
||
Hey Ryan, would you be interested in taking this one as well? It is slightly more involved than the previous one since it has a depedency.
Flags: needinfo?(leakey94)
Assignee | ||
Comment 5•6 years ago
|
||
(In reply to Alessio Placitelli [:Dexter] from comment #4) > Hey Ryan, would you be interested in taking this one as well? It is slightly > more involved than the previous one since it has a depedency. Sure Alessio, I'd like to give it a go.
Flags: needinfo?(leakey94) → needinfo?(alessio.placitelli)
Reporter | ||
Updated•6 years ago
|
Assignee: nobody → leakey94
Flags: needinfo?(alessio.placitelli)
Reporter | ||
Updated•6 years ago
|
Mentor: alessio.placitelli
Assignee | ||
Comment 6•6 years ago
|
||
Once "histogram_tools.py" has been renamed and all textual occurrences have been updated, how can I update the python_moztelemetry script to reflect the changes?
Flags: needinfo?(alessio.placitelli)
Reporter | ||
Comment 7•6 years ago
|
||
(In reply to Ryan Leake from comment #6) > Once "histogram_tools.py" has been renamed and all textual occurrences have > been updated, how can I update the python_moztelemetry script to reflect the > changes? You can file a PR against https://github.com/mozilla/python_moztelemetry/ to change these files: https://github.com/mozilla/python_moztelemetry/search?utf8=%E2%9C%93&q=histogram_tools.py&type=
Flags: needinfo?(alessio.placitelli)
Assignee | ||
Comment 8•6 years ago
|
||
Thanks Alessio. Do the files in python_moztelemetry need renaming themselves( e.g. update_histogram_tools renamed to update_parse_histogram)?
Flags: needinfo?(alessio.placitelli)
Reporter | ||
Comment 9•6 years ago
|
||
(In reply to Ryan Leake from comment #8) > Thanks Alessio. Do the files in python_moztelemetry need renaming > themselves( e.g. update_histogram_tools renamed to update_parse_histogram)? Yes please. Let's also change the README file if needed.
Flags: needinfo?(alessio.placitelli)
Assignee | ||
Comment 10•6 years ago
|
||
Do the changes to python_moztelemetry look ok? https://github.com/RyanLeake/python_moztelemetry/commit/c4265ddd79a908d1e8486dd910e7ff663b4b6544
Flags: needinfo?(alessio.placitelli)
Reporter | ||
Comment 11•6 years ago
|
||
(In reply to Ryan Leake from comment #10) > Do the changes to python_moztelemetry look ok? > https://github.com/RyanLeake/python_moztelemetry/commit/ > c4265ddd79a908d1e8486dd910e7ff663b4b6544 From a quick look they make sense. Let's make a PR so that I can take a better look :-)
Flags: needinfo?(alessio.placitelli)
Comment 12•6 years ago
|
||
Comment hidden (mozreview-request) |
Reporter | ||
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8931640 [details] Bug 1419761 - Rename 'histogram_tools.py' to 'parse_histograms.py'. https://reviewboard.mozilla.org/r/202808/#review208136 This looks good, thank you!
Attachment #8931640 -
Flags: review?(alessio.placitelli) → review+
Reporter | ||
Comment 15•6 years ago
|
||
Ryan, looks like the changes are failing code linting: https://treeherder.mozilla.org/#/jobs?repo=try&revision=194823f0a3ba65b1343f99ce383aceac275c5ebf&selectedJob=147413387 Could you please fix it and update your patch? If you want to check for linting errors locally, you can run the following command: ./mach lint -l flake8 toolkit/components/telemetry
Flags: needinfo?(leakey94)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•6 years ago
|
||
(In reply to Alessio Placitelli [:Dexter] from comment #15) > Ryan, looks like the changes are failing code linting: > > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=194823f0a3ba65b1343f99ce383aceac275c5ebf&selectedJob=1 > 47413387 > > Could you please fix it and update your patch? If you want to check for > linting errors locally, you can run the following command: > > ./mach lint -l flake8 toolkit/components/telemetry Should be good to go now!
Flags: needinfo?(leakey94) → needinfo?(alessio.placitelli)
Comment 19•6 years ago
|
||
Pushed by alessio.placitelli@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c5d69465077c Rename 'histogram_tools.py' to 'parse_histograms.py'. r=Dexter
Comment 20•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c5d69465077c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 21•6 years ago
|
||
(In reply to Alessio Placitelli [:Dexter] from comment #18) > Thank you! DO YOU HAVE SOME EASY BUGS I KNOW JAVA, C, A LITTLE ANDROID
Reporter | ||
Comment 22•6 years ago
|
||
(In reply to Dailin Zhang from comment #21) > (In reply to Alessio Placitelli [:Dexter] from comment #18) > > Thank you! > > DO YOU HAVE SOME EASY BUGS > I KNOW JAVA, C, A LITTLE ANDROID Please have a look at http://www.joshmatthews.net/bugsahoy/ or http://whatcanidoformozilla.org/
You need to log in
before you can comment on or make changes to this bug.
Description
•