[Merge] lp:~didrocks/ubuntu-release-upgrader/add_telemetry into lp:ubuntu-release-upgrader
Brian Murray
brian at ubuntu.com
Wed May 9 19:22:30 UTC 2018
Review: Needs Information
Keep in mind that it is possible to from one release to multiple releases e.g. from xenial to artful or bionic. So without knowing the database structure I think it would make sense to also record the "To" release. You can find this in DistUpgradeController.py.
I'm also curious about how this information will be used because the use cases might benefit from gathering additional information like the version of release upgrader being used or environmental variables set.
Otherwise there are a couple of typos and one nitpick to have a look at.
Diff comments:
>
> === modified file 'DistUpgrade/DistUpgradeViewGtk3.py'
> --- DistUpgrade/DistUpgradeViewGtk3.py 2016-02-23 17:06:42 +0000
> +++ DistUpgrade/DistUpgradeViewGtk3.py 2018-05-04 14:04:01 +0000
> @@ -598,44 +601,45 @@
> def updateStatus(self, msg):
> self.label_status.set_text("%s" % msg)
> def hideStep(self, step):
> - image = getattr(self,"image_step%i" % step)
> - label = getattr(self,"label_step%i" % step)
> - #arrow = getattr(self,"arrow_step%i" % step)
> + image = getattr(self,"image_step%i" % step.value)
> + label = getattr(self, "label_step%i" % step.value)
A space seems to have been added here.
> + #arrow = getattr(self,"arrow_step%i" % step.value)
> image.hide()
> label.hide()
> def showStep(self, step):
> - image = getattr(self,"image_step%i" % step)
> - label = getattr(self,"label_step%i" % step)
> + image = getattr(self,"image_step%i" % step.value)
> + label = getattr(self, "label_step%i" % step.value)
A space was also added here.
> image.show()
> label.show()
> def abort(self):
> size = Gtk.IconSize.MENU
> step = self.prev_step
> - if step > 0:
> - image = getattr(self,"image_step%i" % step)
> - arrow = getattr(self,"arrow_step%i" % step)
> + if step:
> + image = getattr(self,"image_step%i" % step.value)
> + arrow = getattr(self,"arrow_step%i" % step.value)
> image.set_from_stock(Gtk.STOCK_CANCEL, size)
> image.show()
> arrow.hide()
> def setStep(self, step):
> + super(DistUpgradeViewGtk3, self).setStep(step)
> if self.icontheme.rescan_if_needed():
> logging.debug("icon theme changed, re-reading")
> # first update the "previous" step as completed
> size = Gtk.IconSize.MENU
> attrlist=Pango.AttrList()
> if self.prev_step:
> - image = getattr(self,"image_step%i" % self.prev_step)
> - label = getattr(self,"label_step%i" % self.prev_step)
> - arrow = getattr(self,"arrow_step%i" % self.prev_step)
> + image = getattr(self,"image_step%i" % self.prev_step.value)
> + label = getattr(self,"label_step%i" % self.prev_step.value)
> + arrow = getattr(self,"arrow_step%i" % self.prev_step.value)
> label.set_property("attributes",attrlist)
> image.set_from_stock(Gtk.STOCK_APPLY, size)
> image.show()
> arrow.hide()
> self.prev_step = step
> # show the an arrow for the current step and make the label bold
> - image = getattr(self,"image_step%i" % step)
> - label = getattr(self,"label_step%i" % step)
> - arrow = getattr(self,"arrow_step%i" % step)
> + image = getattr(self,"image_step%i" % step.value)
> + label = getattr(self,"label_step%i" % step.value)
> + arrow = getattr(self,"arrow_step%i" % step.value)
> # check if that step was not hidden with hideStep()
> if not label.get_property("visible"):
> return
>
> === modified file 'DistUpgrade/DistUpgradeViewKDE.py'
> --- DistUpgrade/DistUpgradeViewKDE.py 2018-01-22 11:12:39 +0000
> +++ DistUpgrade/DistUpgradeViewKDE.py 2018-05-04 14:04:01 +0000
> @@ -733,15 +736,15 @@
> self.window_main.label_status.setText(msg)
>
> def hideStep(self, step):
> - image = getattr(self.window_main,"image_step%i" % step)
> - label = getattr(self.window_main,"label_step%i" % step)
> + image = getattr(self.window_main,"image_step%i" % step.value)
> + label = getattr(self.window_main, "label_step%i" % step.value)
A space was also added here.
> image.hide()
> label.hide()
>
> def abort(self):
> step = self.prev_step
> - if step > 0:
> - image = getattr(self.window_main,"image_step%i" % step)
> + if step:
> + image = getattr(self.window_main,"image_step%i" % step.value)
> cancelIcon = _icon("dialog-cancel",
> fallbacks=["/usr/share/icons/oxygen/16x16/actions/dialog-cancel.png",
> "/usr/lib/kde4/share/icons/oxygen/16x16/actions/dialog-cancel.png",
>
> === added file 'DistUpgrade/telemetry.py'
> --- DistUpgrade/telemetry.py 1970-01-01 00:00:00 +0000
> +++ DistUpgrade/telemetry.py 2018-05-04 14:04:01 +0000
> @@ -0,0 +1,80 @@
> +# -*- coding: utf-8; Mode: Python; indent-tabs-mode: nil; tab-width: 4 -*-
> +
> +# Copyright (C) 2018 Canonical Ltd.
> +#
> +# Functions useful for the final install.py script and for ubiquity
> +# plugins to use
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +
> +
> +import logging
> +import json
> +import os
> +import stat
> +import subprocess
> +import time
> +
> +
> +def get():
> + """Return a singleton _Telemetry instance."""
> + if _Telemetry._telemetry is None:
> + _Telemetry._telemetry = _Telemetry()
> + return _Telemetry._telemetry
> +
> +
> +class _Telemetry():
> +
> + _telemetry = None
> +
> + def __init__(self):
> + self._metrics = {}
> + self._stages_hist = {}
> + self._start_time = time.time()
> + self._metrics["From"] = subprocess.Popen(
> + ["lsb_release", "-r", "-s"], stdout=subprocess.PIPE,
The release upgrader code uses and refers to releases by their codename so I think using 'lsb_release -c -s' would be more consistent.
> + universal_newlines=True).communicate()[0].strip()
> + self.add_stage('start')
> + self._dest_path = '/var/log/upgrade/telemetry'
> +
> + def add_stage(self, stage_name):
> + """Record installer stage with current time"""
> + self._stages_hist[int(time.time() - self._start_time)] = stage_name
> +
> + def set_updater_type(self, updater_type):
> + """Record updater type"""
> + self._metrics['Type'] = updater_type
> +
> + def done(self):
> + """Close telemetry collection
> +
> + Save to destination file"""
> +
> + self._metrics['Stages'] = self._stages_hist
> +
> + target_dir = os.path.dirname(self._dest_path)
> + try:
> + if not os.path.exists(target_dir):
> + os.makedirs(target_dir)
> + with open(self._dest_path, 'w') as f:
> + json.dump(self._metrics, f)
> + os.chmod(self._dest_path,
> + stat.S_IRUSR | stat.S_IWUSR |
> + stat.S_IRGRP | stat.S_IROTH)
> + except OSError as e:
> + logging.warning("Exception while storing telemetry data: " +
> + str(e))
> +
> +# vim:ai:et:sts=4:tw=80:sw=4:
--
https://code.launchpad.net/~didrocks/ubuntu-release-upgrader/add_telemetry/+merge/345088
Your team Ubuntu Core Development Team is subscribed to branch lp:ubuntu-release-upgrader.
More information about the Ubuntu-reviews
mailing list