Page MenuHomePhabricator

Move Analytics Report Updater to Python 3
Closed, ResolvedPublic8 Story Points

Details

Related Gerrit Patches:
analytics/reportupdater : mastergraphite.py: encode a text string before socket.send
operations/puppet : productionreportupdater::job: use python3 in the systemd timer
analytics/reportupdater : masterMove codebase to python3
analytics/reportupdater : masterMove reportupdater_test5 to python3

Event Timeline

elukey triaged this task as Medium priority.Sep 18 2018, 4:18 PM
elukey created this task.
mforns lowered the priority of this task from Medium to Low.Apr 22 2019, 4:00 PM

Change 537267 had a related patch set uploaded (by Elukey; owner: Elukey):
[analytics/reportupdater@master] Move reportupdater_test5 to python3

https://gerrit.wikimedia.org/r/537267

Change 537268 had a related patch set uploaded (by Elukey; owner: Elukey):
[analytics/reportupdater@master] Move codebase to python3

https://gerrit.wikimedia.org/r/537268

Change 537267 abandoned by Elukey:
Move reportupdater_test5 to python3

https://gerrit.wikimedia.org/r/537267

This is a bit harder than expected, since tests will need some adjustment to:

  1. run tox with python 3.7 in jenkins
  2. encode/decode properly strings to avoid failures

https://gerrit.wikimedia.org/r/537267 could be used as baseline, I have ran the python 2to3 tool on all files.

elukey raised the priority of this task from Low to Medium.Sep 17 2019, 7:33 AM

https://gerrit.wikimedia.org/r/#/c/analytics/reportupdater/+/537268/ is ready for a first review, testing with real data has not been done yet.

Jenkins already supports python3, it is just a matter of changing the tox.ini file!

elukey claimed this task.Sep 24 2019, 6:58 AM
elukey added a project: Analytics-Kanban.
elukey moved this task from Next Up to In Code Review on the Analytics-Kanban board.

Change 539021 had a related patch set uploaded (by Elukey; owner: Elukey):
[operations/puppet@production] reportupdater::job: use python3 in the systemd timer

https://gerrit.wikimedia.org/r/539021

Change 537268 merged by Elukey:
[analytics/reportupdater@master] Move codebase to python3

https://gerrit.wikimedia.org/r/537268

Change 539021 merged by Elukey:
[operations/puppet@production] reportupdater::job: use python3 in the systemd timer

https://gerrit.wikimedia.org/r/539021

elukey set the point value for this task to 8.Sep 25 2019, 6:07 AM

Marcel verified that the code runs correctly with python3, and today I deployed it and updated the systemd timers on stat1006/7. Everything seems working as expected!

elukey moved this task from In Code Review to Done on the Analytics-Kanban board.Sep 25 2019, 8:15 AM
Nuria closed this task as Resolved.Oct 9 2019, 3:49 PM
awight added a subscriber: awight.EditedOct 29 2019, 2:46 PM

This must be my fault, but I thought I would mention it anyway, at least until I understand what's wrong. I get this python2-backcompat-looking error when running reportupdater on a stat machine:

2019-10-29 14:39:17,772 - ERROR - Report "baseline" could not be written because of error: a bytes-like object is required, not 'str'
Traceback (most recent call last):
  File "/srv/reportupdater/reportupdater/reportupdater/writer.py", line 46, in run
    self.record_to_graphite(report, new_dates)
  File "/srv/reportupdater/reportupdater/reportupdater/writer.py", line 185, in record_to_graphite
    self.graphite.record_row(row, report)
  File "/srv/reportupdater/reportupdater/reportupdater/graphite.py", line 64, in record_row
    self.record(metric, value, timestamp)
  File "/srv/reportupdater/reportupdater/reportupdater/graphite.py", line 104, in record
    sock.send('{} {} {}\n'.format(metric, value, timestamp))
TypeError: a bytes-like object is required, not 'str'

Steps to reproduce:

elukey reopened this task as Open.Oct 29 2019, 2:50 PM

@awight this is probably something that we didn't test it, as far as I know we don't use the graphite writer.. the stacktrace makes sense, a text string should not be passed to sock.send().. Will check the code asap!

+1 it does look like a bug. The solution would be as simple as .encode('utf-8') (I ran that locally and it works, at least) however I haven't been able to verify which encoding the Graphite plaintext protocol expects, so a non-ascii character might cause meltdown.

+1 it does look like a bug. The solution would be as simple as .encode('utf-8') (I ran that locally and it works, at least) however I haven't been able to verify which encoding the Graphite plaintext protocol expects, so a non-ascii character might cause meltdown.

Yep the fix looks good, if you want to file a code review I'll review/merge it! Please also be aware that graphite is in the process of being deprecated (in a couple of years probably, but still best to know it).

Change 546971 had a related patch set uploaded (by Awight; owner: Awight):
[analytics/reportupdater@master] Leftover py2-3 glitch: encode a string to bytes

https://gerrit.wikimedia.org/r/546971

Change 546971 merged by jenkins-bot:
[analytics/reportupdater@master] graphite.py: encode a text string before socket.send

https://gerrit.wikimedia.org/r/546971

Nuria closed this task as Resolved.Nov 7 2019, 11:05 PM