All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail
@ 2019-08-04 10:53 Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 1/9] utils/daily-mail: make the script flake8 compliant Victor Huesca
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

This patch-series adds several improvements to the daily-mail script
in order to keep it clean when additional functionalities will be
added (outdated packages notification and gitlab-ci failed tests
notification).


Victor Huesca (9):
  utils/daily-mail: make the script flake8 compliant
  utils/daily-mail: replace function's comments by doc-strings
  utils/daily-mail: add a function to shrink strings to a fixed size
  utils/daily-mail: remove the unnecessary 'get_notification_for_dev'
    function
  utils/daily-mail: improve code readability
  utils/daily-mail: change the URL length formatting to reflect the
    actual URL length
  utils/daily-mail: change variable name to be more explicit
  utils/daily-mail: replace '%' style format strings by the newer '{}'
    version
  utils/daily-mail: add a header for the build_results section

 utils/daily-mail | 221 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 143 insertions(+), 78 deletions(-)

-- 
2.21.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 1/9] utils/daily-mail: make the script flake8 compliant
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 2/9] utils/daily-mail: replace function's comments by doc-strings Victor Huesca
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

This patch apply some coding styles changes to make the script flake8
pass the flake8 tests.
The goal is to have a cleaner code for the following patches.

This patch fixes the following:
- missing blank lines between functions/methods
- deprecated functions (dict.has_key)
- trailing whitespaces
- whitespace after '(', '{' and '['
- whitespace before ')', '}' and ']'
- continuation line under/over indented
- line too long (limit set to 132 characters)

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 432f7e5..6db7f3b 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -13,13 +13,14 @@ import csv
 from collections import defaultdict
 
 sys.path.append(os.path.join(localconfig.brbase, "utils"))
-import getdeveloperlib
+import getdeveloperlib  # noqa: E402
 
 baseurl = "autobuild.buildroot.net"
 http_baseurl = "http://" + baseurl
 
 developers = getdeveloperlib.parse_developers(localconfig.brbase)
 
+
 def get_branches():
     """Returns the list of branches currently tested by the autobuilders."""
     branch_file = os.path.join(os.path.dirname(os.path.realpath(__file__)), "..", "web", "branches")
@@ -30,6 +31,7 @@ def get_branches():
         branches.append(branch[0])
     return branches
 
+
 # Find, for the previous day, the global statistics: number of
 # success, failures, timeouts, and total number of builds.
 def get_overall_stats(db, datestr, branches):
@@ -55,11 +57,13 @@ def get_overall_stats(db, datestr, branches):
         stats[branch] = (success, failures, timeouts, total)
     return stats
 
+
 class Notification:
     def __init__(self):
         self.arch_notifications = defaultdict(list)
         self.package_notifications = defaultdict(list)
 
+
 # Calculate the list of .mk files in the Buildroot source tree, will
 # be used to guess the name of the packages that caused build
 # failures.
@@ -72,8 +76,10 @@ def get_mklist(basepath):
             mklist.append(os.path.splitext(f)[0])
     return mklist
 
+
 mklist = get_mklist(localconfig.brbase)
 
+
 def get_notification_for_dev(notifications, dev):
     if dev in notifications:
         return notifications[dev]
@@ -82,6 +88,7 @@ def get_notification_for_dev(notifications, dev):
         notifications[dev] = n
         return n
 
+
 # Add to the notifications{} dict notifications that are related to
 # architecture "maintainers".
 def add_arch_notification(branch, notifications, build_result):
@@ -92,6 +99,7 @@ def add_arch_notification(branch, notifications, build_result):
         n = get_notification_for_dev(notifications, dev)
         n.arch_notifications[branch].append(build_result)
 
+
 # Given a failure reason as provided by the autobuilders, tries to
 # find the corresponding package by stripping progressively the last
 # "-<something>" parts of the failure reason. A failure reason like
@@ -112,14 +120,17 @@ def find_package(reason):
             return reason
     return None
 
+
 ORPHAN_DEVELOPER = "Arnout Vandecappelle <arnout@mind.be>"
 
+
 def get_orphan_developer():
     for dev in developers:
         if dev.name == ORPHAN_DEVELOPER:
             return dev
     return None
 
+
 # Add to the notifications{} dict notifications that are related to
 # package "maintainers".
 def add_package_notification(branch, notifications, build_result):
@@ -139,6 +150,7 @@ def add_package_notification(branch, notifications, build_result):
         n.package_notifications[branch].append(build_result)
     build_result['orphan'] = orphan
 
+
 def show_results(results, show_status, show_orphan=False):
     contents = ""
     for r in results:
@@ -151,7 +163,7 @@ def show_results(results, show_status, show_orphan=False):
             status_str = "NOK"
         elif status == 2:
             status_str = "TIM"
-        if r.has_key('orphan') and r['orphan']:
+        if 'orphan' in r and r['orphan']:
             orphan_str = "ORPH"
         else:
             orphan_str = ""
@@ -166,6 +178,7 @@ def show_results(results, show_status, show_orphan=False):
             contents += "\n"
     return contents
 
+
 # Send the e-mails to the individual developers
 def developers_email(smtp, branches, notifications, datestr, dry_run):
     for k, v in notifications.iteritems():
@@ -173,16 +186,20 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
         email_from = localconfig.fromaddr
         subject = "[%s] Your build results for %s" % (baseurl, datestr)
         contents = "Hello,\n\n"
-        contents += textwrap.fill("This is the list of Buildroot build failures that occured on %s, and for which you are a registered architecture developer or package developer. Please help us improving the quality of Buildroot by investigating those build failures and sending patches to fix them. Thanks!" % datestr)
+        contents += textwrap.fill("This is the list of Buildroot build failures that occurred on %s, "
+                                  "and for which you are a registered architecture developer or package "
+                                  "developer. Please help us improving the quality of Buildroot by "
+                                  "investigating those build failures and sending patches to fix them. "
+                                  "Thanks!" % datestr)
         contents += "\n\n"
         show_orphan = k.name == ORPHAN_DEVELOPER
 
         for branch in branches:
-            if v.arch_notifications.has_key(branch):
+            if branch in v.arch_notifications:
                 archs = v.arch_notifications[branch]
             else:
                 archs = []
-            if v.package_notifications.has_key(branch):
+            if branch in v.package_notifications:
                 packages = v.package_notifications[branch]
             else:
                 packages = []
@@ -222,6 +239,7 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
             smtp.sendmail(email_from, to, msg.as_string())
             print "To: %s" % k.name
 
+
 def global_email_branch_result(results, results_by_reason, branch):
     contents = "Results for branch '%s'\n" % branch
     contents += "=====================" + "=" * len(branch) + "\n\n"
@@ -240,6 +258,7 @@ def global_email_branch_result(results, results_by_reason, branch):
     contents += "\n"
     return contents
 
+
 # Send the global e-mail to the mailing list
 def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
     to = "buildroot at buildroot.org"
@@ -280,17 +299,20 @@ def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
         smtp.sendmail(email_from, [to], msg.as_string())
         print "To: buildroot at buildroot.net"
 
+
 # Get the list of build failures for the past day
 def get_build_results(db, datestr, branches):
     results = {}
     for branch in branches:
         db.query("""select * from results
-        where date(builddate) = '%s' and status != 0 and branch = '%s' order by reason""" % \
-                 (datestr, branch))
+        where date(builddate) = '%s'
+        and status != 0 and branch = '%s'
+        order by reason""" % (datestr, branch))
         r = db.use_result()
         results[branch] = r.fetch_row(how=1, maxrows=0)
     return results
 
+
 def get_build_results_grouped_by_reason(db, datestr, branches):
     results_by_reason = {}
     for branch in branches:
@@ -301,6 +323,7 @@ def get_build_results_grouped_by_reason(db, datestr, branches):
         results_by_reason[branch] = r.fetch_row(how=1, maxrows=0)
     return results_by_reason
 
+
 # Prepare the notifications{} dict for the notifications to individual
 # developers, based on architecture developers and package
 # developers
@@ -315,6 +338,7 @@ def calculate_notifications(results):
             add_package_notification(branch, notifications, result)
     return notifications
 
+
 def __main__():
     yesterday = date.today() - timedelta(1)
     yesterday_str = yesterday.strftime('%Y-%m-%d')
@@ -339,4 +363,5 @@ def __main__():
                  overall_stats, dry_run)
     smtp.quit()
 
+
 __main__()
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 2/9] utils/daily-mail: replace function's comments by doc-strings
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 1/9] utils/daily-mail: make the script flake8 compliant Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 3/9] utils/daily-mail: add a function to shrink strings to a fixed size Victor Huesca
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

Since these comments describe what their corresponding function does, it
makes sense to use docstring instead of regular comment. This improve a
bit the readability.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 62 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 6db7f3b..0774242 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -32,9 +32,11 @@ def get_branches():
     return branches
 
 
-# Find, for the previous day, the global statistics: number of
-# success, failures, timeouts, and total number of builds.
 def get_overall_stats(db, datestr, branches):
+    '''
+    Find, for the previous day, the global statistics: number of
+    success, failures, timeouts, and total number of builds.
+    '''
     stats = {}
     for branch in branches:
         db.query("""select status,count(id) as count from results
@@ -64,10 +66,12 @@ class Notification:
         self.package_notifications = defaultdict(list)
 
 
-# Calculate the list of .mk files in the Buildroot source tree, will
-# be used to guess the name of the packages that caused build
-# failures.
 def get_mklist(basepath):
+    '''
+    Calculate the list of .mk files in the Buildroot source tree, will
+    be used to guess the name of the packages that caused build
+    failures.
+    '''
     mklist = []
     for root, dirs, files in os.walk(basepath):
         for f in files:
@@ -89,9 +93,11 @@ def get_notification_for_dev(notifications, dev):
         return n
 
 
-# Add to the notifications{} dict notifications that are related to
-# architecture "maintainers".
 def add_arch_notification(branch, notifications, build_result):
+    '''
+    Add to the notifications{} dict notifications that are related to
+    architecture "maintainers".
+    '''
     arch = build_result['arch']
     for dev in developers:
         if arch not in dev.architectures:
@@ -100,14 +106,16 @@ def add_arch_notification(branch, notifications, build_result):
         n.arch_notifications[branch].append(build_result)
 
 
-# Given a failure reason as provided by the autobuilders, tries to
-# find the corresponding package by stripping progressively the last
-# "-<something>" parts of the failure reason. A failure reason like
-# "qt5location-5.6.1-1" will first attempt to find a package named
-# "qt5location-5.6.1" (which will not find any match) and then attempt
-# to find a package named "qt5location" (which will match an existing
-# package).
 def find_package(reason):
+    '''
+    Given a failure reason as provided by the autobuilders, tries to
+    find the corresponding package by stripping progressively the last
+    "-<something>" parts of the failure reason. A failure reason like
+    "qt5location-5.6.1-1" will first attempt to find a package named
+    "qt5location-5.6.1" (which will not find any match) and then attempt
+    to find a package named "qt5location" (which will match an existing
+    package).
+    '''
     if reason == "unknown":
         return
     # Strip host- prefix so that host packages can match
@@ -131,9 +139,11 @@ def get_orphan_developer():
     return None
 
 
-# Add to the notifications{} dict notifications that are related to
-# package "maintainers".
 def add_package_notification(branch, notifications, build_result):
+    '''
+    Add to the notifications{} dict notifications that are related to
+    package "maintainers".
+    '''
     pkg = find_package(build_result['reason'])
     if not pkg:
         return
@@ -179,8 +189,10 @@ def show_results(results, show_status, show_orphan=False):
     return contents
 
 
-# Send the e-mails to the individual developers
 def developers_email(smtp, branches, notifications, datestr, dry_run):
+    '''
+    Send the e-mails to the individual developers
+    '''
     for k, v in notifications.iteritems():
         to = k.name
         email_from = localconfig.fromaddr
@@ -259,8 +271,10 @@ def global_email_branch_result(results, results_by_reason, branch):
     return contents
 
 
-# Send the global e-mail to the mailing list
 def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
+    '''
+    Send the global e-mail to the mailing list
+    '''
     to = "buildroot at buildroot.org"
     email_from = localconfig.fromaddr
     subject = "[%s] Build results for %s" % (baseurl, datestr)
@@ -300,8 +314,10 @@ def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
         print "To: buildroot at buildroot.net"
 
 
-# Get the list of build failures for the past day
 def get_build_results(db, datestr, branches):
+    '''
+    Get the list of build failures for the past day
+    '''
     results = {}
     for branch in branches:
         db.query("""select * from results
@@ -324,10 +340,12 @@ def get_build_results_grouped_by_reason(db, datestr, branches):
     return results_by_reason
 
 
-# Prepare the notifications{} dict for the notifications to individual
-# developers, based on architecture developers and package
-# developers
 def calculate_notifications(results):
+    '''
+    Prepare the notifications{} dict for the notifications to individual
+    developers, based on architecture developers, package developers,
+    defconfig developers and runtime developers.
+    '''
     notifications = {}
     for branch in results.keys():
         for result in results[branch]:
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 3/9] utils/daily-mail: add a function to shrink strings to a fixed size
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 1/9] utils/daily-mail: make the script flake8 compliant Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 2/9] utils/daily-mail: replace function's comments by doc-strings Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 4/9] utils/daily-mail: remove the unnecessary 'get_notification_for_dev' function Victor Huesca
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

This patch adds a function to shrink a string to a fixed length while
appending '...' to the remaining part. This allows to factorize a bit
the formatting part and will allow to easily add this kind of length
restriction in future kind of notification.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 0774242..f6a55a6 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -11,6 +11,7 @@ from datetime import date, timedelta
 import localconfig
 import csv
 from collections import defaultdict
+import math
 
 sys.path.append(os.path.join(localconfig.brbase, "utils"))
 import getdeveloperlib  # noqa: E402
@@ -161,13 +162,38 @@ def add_package_notification(branch, notifications, build_result):
     build_result['orphan'] = orphan
 
 
+def shrink_str(string, length, allign='right', fill='...'):
+    '''
+    Returns the `string` shrinked to fit `length` characters and with `fill`
+    added on `allign`.
+
+    :Example:
+
+    >>> shrink_str('This is a very long string!', 10)
+    'This is...'
+
+    >>> shrink_str('This is a very long string!', 20, allign='center', fill='[...]')
+    'This is[...] string!'
+    '''
+    if len(fill) > length:
+        raise ValueError('`length` cannot be shorter than `fill`')
+    if allign == 'right':
+        return string[:length-len(fill)] + fill if len(string) > length else string
+    elif allign == 'left':
+        return fill + string[len(string)-length+len(fill):] if len(string) > length else string
+    elif allign == 'center':
+        lcut = math.floor((length - len(fill)) / 2)
+        rcut = len(string) - math.ceil((length - len(fill)) / 2)
+        return string[:lcut] + fill + string[rcut:] if len(string) > length else string
+    else:
+        raise ValueError('allign must be either `left`, `right` or `center`')
+
+
 def show_results(results, show_status, show_orphan=False):
     contents = ""
     for r in results:
         arch = r['arch']
-        reason = r['reason']
-        if len(reason) > 30:
-            reason = reason[0:27] + "..."
+        reason = shrink_str(r['reason'], 30)
         status = int(r['status'])
         if status == 1:
             status_str = "NOK"
@@ -258,9 +284,7 @@ def global_email_branch_result(results, results_by_reason, branch):
     contents += "Classification of failures by reason\n"
     contents += "------------------------------------\n\n"
     for r in results_by_reason:
-        reason = r['reason']
-        if len(reason) > 30:
-            reason = reason[0:27] + "..."
+        reason = shrink_str(r['reason'], 30)
         count = int(r['reason_count'])
         contents += "%30s | %-2d\n" % (reason, count)
     contents += "\n\n"
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 4/9] utils/daily-mail: remove the unnecessary 'get_notification_for_dev' function
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (2 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 3/9] utils/daily-mail: add a function to shrink strings to a fixed size Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 5/9] utils/daily-mail: improve code readability Victor Huesca
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

Python dictionaries provide a way to return a value associated to a key
while adding the corresponding entry in case the key doesn't exists yet
(it acts like a default_dict from itertools).
Since the 'get_notification_for_dev' does exactly that, this patch
remove this function and replace it by the build-in
`dict.setdefault(key, value)`.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index f6a55a6..12f909e 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -85,15 +85,6 @@ def get_mklist(basepath):
 mklist = get_mklist(localconfig.brbase)
 
 
-def get_notification_for_dev(notifications, dev):
-    if dev in notifications:
-        return notifications[dev]
-    else:
-        n = Notification()
-        notifications[dev] = n
-        return n
-
-
 def add_arch_notification(branch, notifications, build_result):
     '''
     Add to the notifications{} dict notifications that are related to
@@ -103,7 +94,7 @@ def add_arch_notification(branch, notifications, build_result):
     for dev in developers:
         if arch not in dev.architectures:
             continue
-        n = get_notification_for_dev(notifications, dev)
+        n = notifications.setdefault(dev, Notification())
         n.arch_notifications[branch].append(build_result)
 
 
@@ -153,11 +144,11 @@ def add_package_notification(branch, notifications, build_result):
         if pkg not in dev.packages:
             continue
         orphan = False
-        n = get_notification_for_dev(notifications, dev)
+        n = notifications.setdefault(dev, Notification())
         n.package_notifications[branch].append(build_result)
     if orphan:
         dev = get_orphan_developer()
-        n = get_notification_for_dev(notifications, dev)
+        n = notifications.setdefault(dev, Notification())
         n.package_notifications[branch].append(build_result)
     build_result['orphan'] = orphan
 
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 5/9] utils/daily-mail: improve code readability
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (3 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 4/9] utils/daily-mail: remove the unnecessary 'get_notification_for_dev' function Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 6/9] utils/daily-mail: change the URL length formatting to reflect the actual URL length Victor Huesca
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

Python dictionaries provide a nice way to get a given value from a dict
while returning a default one in case the key doesn't exists.
This function has the particularity to not modify the dictionary.

This patch replaces code chunks that does this by this 'dict.get(key,
default)'.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 12f909e..c489fc1 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -190,10 +190,7 @@ def show_results(results, show_status, show_orphan=False):
             status_str = "NOK"
         elif status == 2:
             status_str = "TIM"
-        if 'orphan' in r and r['orphan']:
-            orphan_str = "ORPH"
-        else:
-            orphan_str = ""
+        orphan_str = 'ORPH' if r.get('orphan') else ''
         url = http_baseurl + "/results/" + r['identifier']
         if show_status:
             contents += "%12s | %30s | %3s | %40s" % (arch, reason, status_str, url)
@@ -224,14 +221,8 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
         show_orphan = k.name == ORPHAN_DEVELOPER
 
         for branch in branches:
-            if branch in v.arch_notifications:
-                archs = v.arch_notifications[branch]
-            else:
-                archs = []
-            if branch in v.package_notifications:
-                packages = v.package_notifications[branch]
-            else:
-                packages = []
+            archs = v.arch_notifications.get(branch, [])
+            packages = v.package_notifications.get(branch, [])
 
             if len(archs) == 0 and len(packages) == 0:
                 continue
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 6/9] utils/daily-mail: change the URL length formatting to reflect the actual URL length
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (4 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 5/9] utils/daily-mail: improve code readability Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 7/9] utils/daily-mail: change variable name to be more explicit Victor Huesca
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

The formating length specifies a 40 characters long string but the
actual length is 79 characters long. This patch updates the lenght to
79.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index c489fc1..4cc5345 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -193,9 +193,9 @@ def show_results(results, show_status, show_orphan=False):
         orphan_str = 'ORPH' if r.get('orphan') else ''
         url = http_baseurl + "/results/" + r['identifier']
         if show_status:
-            contents += "%12s | %30s | %3s | %40s" % (arch, reason, status_str, url)
+            contents += "%12s | %30s | %3s | %79s" % (arch, reason, status_str, url)
         else:
-            contents += "%12s | %30s | %40s" % (arch, reason, url)
+            contents += "%12s | %30s | %79s" % (arch, reason, url)
         if show_orphan:
             contents += " | %4s\n" % (orphan_str)
         else:
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 7/9] utils/daily-mail: change variable name to be more explicit
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (5 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 6/9] utils/daily-mail: change the URL length formatting to reflect the actual URL length Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 8/9] utils/daily-mail: replace '%' style format strings by the newer '{}' version Victor Huesca
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

Replace 'k' and 'v' variables in favor of 'dev' and 'notif'.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 4cc5345..6786caa 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -207,8 +207,8 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
     '''
     Send the e-mails to the individual developers
     '''
-    for k, v in notifications.iteritems():
-        to = k.name
+    for dev, notif in notifications.iteritems():
+        to = dev.name
         email_from = localconfig.fromaddr
         subject = "[%s] Your build results for %s" % (baseurl, datestr)
         contents = "Hello,\n\n"
@@ -218,11 +218,11 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
                                   "investigating those build failures and sending patches to fix them. "
                                   "Thanks!" % datestr)
         contents += "\n\n"
-        show_orphan = k.name == ORPHAN_DEVELOPER
+        show_orphan = dev.name == ORPHAN_DEVELOPER
 
         for branch in branches:
-            archs = v.arch_notifications.get(branch, [])
-            packages = v.package_notifications.get(branch, [])
+            archs = notif.arch_notifications.get(branch, [])
+            packages = notif.package_notifications.get(branch, [])
 
             if len(archs) == 0 and len(packages) == 0:
                 continue
@@ -257,7 +257,7 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
             msg['From'] = email_from
             msg['Date'] = formatdate()
             smtp.sendmail(email_from, to, msg.as_string())
-            print "To: %s" % k.name
+            print "To: %s" % dev.name
 
 
 def global_email_branch_result(results, results_by_reason, branch):
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 8/9] utils/daily-mail: replace '%' style format strings by the newer '{}' version
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (6 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 7/9] utils/daily-mail: change variable name to be more explicit Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 9/9] utils/daily-mail: add a header for the build_results section Victor Huesca
  2019-08-04 16:34 ` [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Thomas Petazzoni
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

Python provides several ways to perform string formatting. The 1st is C
style inspired and use the '%' operator, this is the original way to
create format strings in python. The second way -- which is the standard
in python3 -- is the '.format()' method, this version is more flexible,
removes the strong type constraints and allow more format styles. This
method is also noted as the preferred way over the '%' formatting [1].

In order to introduce new kind of notification, this patch update the
current format strings to this new standard.

[1] https://docs.python.org/2/library/stdtypes.html#str.format

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 51 ++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/utils/daily-mail b/utils/daily-mail
index 6786caa..7af1933 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -41,7 +41,7 @@ def get_overall_stats(db, datestr, branches):
     stats = {}
     for branch in branches:
         db.query("""select status,count(id) as count from results
-        where date(builddate) = '%s' and branch = '%s' group by status""" % (datestr, branch))
+        where date(builddate) = '{}' and branch = '{}' group by status""".format(datestr, branch))
         r = db.use_result()
         result = dict(r.fetch_row(maxrows=0))
         if '0' in result:
@@ -193,11 +193,11 @@ def show_results(results, show_status, show_orphan=False):
         orphan_str = 'ORPH' if r.get('orphan') else ''
         url = http_baseurl + "/results/" + r['identifier']
         if show_status:
-            contents += "%12s | %30s | %3s | %79s" % (arch, reason, status_str, url)
+            contents += "{:^12} | {:^30} | {:^3} | {:^79}".format(arch, reason, status_str, url)
         else:
-            contents += "%12s | %30s | %79s" % (arch, reason, url)
+            contents += "{:^12} | {:^30} | {:^79}".format(arch, reason, url)
         if show_orphan:
-            contents += " | %4s\n" % (orphan_str)
+            contents += " | {:^4}\n".format(orphan_str)
         else:
             contents += "\n"
     return contents
@@ -210,13 +210,13 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
     for dev, notif in notifications.iteritems():
         to = dev.name
         email_from = localconfig.fromaddr
-        subject = "[%s] Your build results for %s" % (baseurl, datestr)
+        subject = "[{}] Your build results for {}".format(baseurl, datestr)
         contents = "Hello,\n\n"
-        contents += textwrap.fill("This is the list of Buildroot build failures that occurred on %s, "
+        contents += textwrap.fill("This is the list of Buildroot build failures that occurred on {}, "
                                   "and for which you are a registered architecture developer or package "
                                   "developer. Please help us improving the quality of Buildroot by "
                                   "investigating those build failures and sending patches to fix them. "
-                                  "Thanks!" % datestr)
+                                  "Thanks!".format(datestr))
         contents += "\n\n"
         show_orphan = dev.name == ORPHAN_DEVELOPER
 
@@ -227,7 +227,7 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
             if len(archs) == 0 and len(packages) == 0:
                 continue
 
-            contents += "Results for the '%s' branch\n" % branch
+            contents += "Results for the '{}' branch\n".format(branch)
             contents += "=========================" + "=" * len(branch) + "\n\n"
 
             if len(archs) != 0:
@@ -244,9 +244,9 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
         contents += http_baseurl
         if dry_run:
             print "====================================================="
-            print "To: %s" % to
-            print "From: %s" % email_from
-            print "Subject: %s" % subject
+            print "To: {}".format(to)
+            print "From: {}".format(email_from)
+            print "Subject: {}".format(subject)
             print
             print contents
             print "====================================================="
@@ -257,18 +257,18 @@ def developers_email(smtp, branches, notifications, datestr, dry_run):
             msg['From'] = email_from
             msg['Date'] = formatdate()
             smtp.sendmail(email_from, to, msg.as_string())
-            print "To: %s" % dev.name
+            print "To: {}".format(dev.name)
 
 
 def global_email_branch_result(results, results_by_reason, branch):
-    contents = "Results for branch '%s'\n" % branch
+    contents = "Results for branch '{}'\n".format(branch)
     contents += "=====================" + "=" * len(branch) + "\n\n"
     contents += "Classification of failures by reason\n"
     contents += "------------------------------------\n\n"
     for r in results_by_reason:
         reason = shrink_str(r['reason'], 30)
         count = int(r['reason_count'])
-        contents += "%30s | %-2d\n" % (reason, count)
+        contents += "{:>30} | {:<2}\n".format(reason, count)
     contents += "\n\n"
     contents += "Detail of failures\n"
     contents += "------------------\n\n"
@@ -283,17 +283,16 @@ def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
     '''
     to = "buildroot@buildroot.org"
     email_from = localconfig.fromaddr
-    subject = "[%s] Build results for %s" % (baseurl, datestr)
+    subject = "[{}] Build results for {}".format(baseurl, datestr)
     contents = "Hello,\n\n"
-    contents += "Build statistics for %s\n" % datestr
+    contents += "Build statistics for {}\n".format(datestr)
     contents += "===============================\n\n"
     contents += "      branch |  OK | NOK | TIM | TOT |\n"
     for branch in sorted(overall.iterkeys()):
         stats = overall[branch]
         if stats[3] == 0:
             continue
-        contents += "  %10s | %3d | %3d | %3d | %3d |\n" % \
-                    (branch, stats[0], stats[1], stats[2], stats[3])
+        contents += "  {:^10} | {:^3} | {:^3} | {:^3} | {:^3} |\n".format(branch, stats[0], stats[1], stats[2], stats[3])
     contents += "\n"
     for branch in results.keys():
         if len(results[branch]) == 0:
@@ -304,9 +303,9 @@ def global_email(smtp, results, results_by_reason, datestr, overall, dry_run):
     contents += http_baseurl
     if dry_run:
         print "====================================================="
-        print "To: %s" % to
-        print "From: %s" % email_from
-        print "Subject: %s" % subject
+        print "To: {}".format(to)
+        print "From: {}".format(email_from)
+        print "Subject: {}".format(subject)
         print
         print contents
         print "====================================================="
@@ -327,9 +326,9 @@ def get_build_results(db, datestr, branches):
     results = {}
     for branch in branches:
         db.query("""select * from results
-        where date(builddate) = '%s'
-        and status != 0 and branch = '%s'
-        order by reason""" % (datestr, branch))
+        where date(builddate) = '{}'
+        and status != 0 and branch = '{}'
+        order by reason""".format(datestr, branch))
         r = db.use_result()
         results[branch] = r.fetch_row(how=1, maxrows=0)
     return results
@@ -339,8 +338,8 @@ def get_build_results_grouped_by_reason(db, datestr, branches):
     results_by_reason = {}
     for branch in branches:
         db.query("""select reason,count(id) as reason_count from results
-        where date(builddate) = '%s' and status != 0 and branch = '%s'
-        group by reason order by reason_count desc, reason""" % (datestr, branch))
+        where date(builddate) = '{}' and status != 0 and branch = '{}'
+        group by reason order by reason_count desc, reason""".format(datestr, branch))
         r = db.use_result()
         results_by_reason[branch] = r.fetch_row(how=1, maxrows=0)
     return results_by_reason
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 9/9] utils/daily-mail: add a header for the build_results section
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (7 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 8/9] utils/daily-mail: replace '%' style format strings by the newer '{}' version Victor Huesca
@ 2019-08-04 10:53 ` Victor Huesca
  2019-08-04 16:34 ` [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Thomas Petazzoni
  9 siblings, 0 replies; 11+ messages in thread
From: Victor Huesca @ 2019-08-04 10:53 UTC (permalink / raw)
  To: buildroot

Adding a header to the autobuild_results table will keep the email clear
when other notifications -- and thus other tables -- will be added.

Signed-off-by: Victor Huesca <victor.huesca@bootlin.com>
---
 utils/daily-mail | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/utils/daily-mail b/utils/daily-mail
index 7af1933..2d5e684 100755
--- a/utils/daily-mail
+++ b/utils/daily-mail
@@ -182,6 +182,23 @@ def shrink_str(string, length, allign='right', fill='...'):
 
 def show_results(results, show_status, show_orphan=False):
     contents = ""
+    # Header
+    if show_status:
+        contents += '{:^12} | {:^30} | {:^3} | {:^79}'.format('arch', 'reason', 'OK?', 'url')
+    else:
+        contents += '{:^12} | {:^30} | {:^79}'.format('arch', 'reason', 'url')
+    if show_orphan:
+        contents += ' | {:^5}'.format('orph?')
+
+    if show_status:
+        contents += '\n{0:-^12}-+-{0:-^30}-+-{0:-^3}-+-{0:-^79}-'.format('')
+    else:
+        contents += '\n{0:-^12}-+-{0:-^30}-+-{0:-^79}-'.format('')
+    if show_orphan:
+        contents += '+-{0:-^5}-'.format('')
+    contents += '\n'
+
+    # Actual data
     for r in results:
         arch = r['arch']
         reason = shrink_str(r['reason'], 30)
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail
  2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
                   ` (8 preceding siblings ...)
  2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 9/9] utils/daily-mail: add a header for the build_results section Victor Huesca
@ 2019-08-04 16:34 ` Thomas Petazzoni
  9 siblings, 0 replies; 11+ messages in thread
From: Thomas Petazzoni @ 2019-08-04 16:34 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun,  4 Aug 2019 12:53:39 +0200
Victor Huesca <victor.huesca@bootlin.com> wrote:

> Victor Huesca (9):
>   utils/daily-mail: make the script flake8 compliant
>   utils/daily-mail: replace function's comments by doc-strings
>   utils/daily-mail: add a function to shrink strings to a fixed size
>   utils/daily-mail: remove the unnecessary 'get_notification_for_dev'
>     function
>   utils/daily-mail: improve code readability
>   utils/daily-mail: change the URL length formatting to reflect the
>     actual URL length
>   utils/daily-mail: change variable name to be more explicit
>   utils/daily-mail: replace '%' style format strings by the newer '{}'
>     version
>   utils/daily-mail: add a header for the build_results section

Series applied. Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-08-04 16:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-04 10:53 [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 1/9] utils/daily-mail: make the script flake8 compliant Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 2/9] utils/daily-mail: replace function's comments by doc-strings Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 3/9] utils/daily-mail: add a function to shrink strings to a fixed size Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 4/9] utils/daily-mail: remove the unnecessary 'get_notification_for_dev' function Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 5/9] utils/daily-mail: improve code readability Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 6/9] utils/daily-mail: change the URL length formatting to reflect the actual URL length Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 7/9] utils/daily-mail: change variable name to be more explicit Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 8/9] utils/daily-mail: replace '%' style format strings by the newer '{}' version Victor Huesca
2019-08-04 10:53 ` [Buildroot] [PATCH buildroot-test 9/9] utils/daily-mail: add a header for the build_results section Victor Huesca
2019-08-04 16:34 ` [Buildroot] [PATCH buildroot-test 0/9] several refactoring of daily-mail Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.