#19 Add remaining data to packager dashboard
Merged 3 years ago by frantisekz. Opened 3 years ago by frantisekz.

@@ -24,5 +24,6 @@ 

  

  PACKAGE_MAINTAINERS_JSON_URL = "https://src.fedoraproject.org/extras/pagure_owner_alias.json"

  ORPHANS_JSON_URL = "https://churchyard.fedorapeople.org/orphans.json"

+ KOSCHEI_API_URL = "https://koschei.fedoraproject.org/api/v1/packages"

  

  EPEL_RELEASES = [7, 8]

file modified
+1
@@ -52,6 +52,7 @@ 

  

      PACKAGE_MAINTAINERS_JSON_URL = 'https://src.fedoraproject.org/extras/pagure_owner_alias.json'

      ORPHANS_JSON_URL = 'https://churchyard.fedorapeople.org/orphans.json'

+     KOSCHEI_API_URL = 'https://koschei.fedoraproject.org/api/v1/packages'

  

      EPEL_RELEASES = [7, 8]

  

@@ -54,8 +54,12 @@ 

      CACHE.register('pagure_groups', dashboard.get_pagure_groups)

      CACHE.register('packages_owners_json', lambda url=app.config['PACKAGE_MAINTAINERS_JSON_URL']: dashboard.get_json(url))

      CACHE.register('orphans_json', lambda url=app.config['ORPHANS_JSON_URL']: dashboard.get_json(url))

+     CACHE.register('koschei_data', dashboard.parse_koschei_data)

+     CACHE.register('health_check_data', dashboard.get_health_check_data)

      CACHE.register('bodhi_overrides', dashboard.get_overrides)

      CACHE.register('bodhi_updates', all_bodhi_updates)

+     CACHE.register('ftbfs_trackers', dashboard.get_ftbfs_trackers)

+     CACHE.register('fti_trackers', dashboard.get_fti_trackers)

  

      CACHE.register('packager-dashboard_user_data_static', dashboard_user_data_static)

      CACHE.register('packager-dashboard_user_data_prs', dashboard_user_data_prs)

@@ -35,12 +35,9 @@ 

          updates[single_release] = libkarma.get_updates(single_release, app.logger)

      return updates

  

- 

- 

  @app.route('/api/v1/packager_dashboard/<user>', methods=['GET'])

  def route_dashboard_user_data(user):

      dashboard.update_user_access_time(user)

- 

      packages_promise = CACHE.async_get('packager-dashboard_user_data_static', user)

      if packages_promise == cache_utils.RefresherNotRegistered:

          static_info = {'status': 404, 'data': None}
@@ -63,19 +60,19 @@ 

              'group_packages': {},

              'primary_packages': [],

              'orphans': {},

+             'fti': {},

              'updates': {},

              'overrides': {},

          }

-     orphans = dashboard.get_orphans(packages["combined"], CACHE.get('orphans_json'))

-     updates = dashboard.get_updates(packages["combined"], CACHE.get('bodhi_updates'))

-     overrides = dashboard.get_user_overrides(packages["combined"], CACHE.get('bodhi_overrides'))

      return {

          'packages': packages["combined"],

          'group_packages': packages["group"],

          'primary_packages': packages["primary"],

-         'orphans': orphans,

-         'updates': updates,

-         'overrides': overrides,

+         'orphans': dashboard.get_orphans(packages["combined"], CACHE.get('orphans_json')),

+         'fails_to_install': dashboard.get_health_check_user_data(user, packages["combined"]),

+         'updates': dashboard.get_updates(packages["combined"], CACHE.get('bodhi_updates')),

+         'overrides': dashboard.get_user_overrides(packages["combined"], CACHE.get('bodhi_overrides')),

+         'koschei': dashboard.get_user_koschei_data(packages["combined"]),

      }

  

  def dashboard_user_data_prs(user):

file modified
+96 -3
@@ -21,6 +21,9 @@ 

  #   Frantisek Zatloukal <fzatlouk@redhat.com>

  #   Josef Skladanka <jskladan@redhat.com>

  

+ from collections import defaultdict

+ from json import JSONDecodeError

+ 

  import json

  import datetime

  import requests
@@ -28,13 +31,12 @@ 

  import bugzilla

  

  from hawkey._hawkey import ValueException

- from json import JSONDecodeError

  from bodhi.client.bindings import BodhiClient

  from urllib3.util.retry import Retry

  from requests.adapters import HTTPAdapter

  from requests.exceptions import ConnectionError

  

- from oraculum import app, db

+ from oraculum import app, db, CACHE

  from oraculum.models.dashboard_users import DashboardUserData

  

  def update_user_access_time(user):
@@ -283,6 +285,24 @@ 

              })

      return data

  

+ def get_fti_trackers():

+     bzapi = bugzilla.Bugzilla("bugzilla.redhat.com")

+     tracker_ids = []

+     releases = [CACHE.get('landing_page')["devel"], CACHE.get('landing_page')["stable"]]

+     for release in releases:

+         bug = bzapi.getbug("F%sFailsToInstall" % release)

+         tracker_ids.append(bug.id)

+     return tracker_ids

+ 

+ def get_ftbfs_trackers():

+     bzapi = bugzilla.Bugzilla("bugzilla.redhat.com")

+     tracker_ids = []

+     releases = [CACHE.get('landing_page')["devel"], CACHE.get('landing_page')["stable"]]

+     for release in releases:

+         bug = bzapi.getbug("F%sFTBFS" % release)

+         tracker_ids.append(bug.id)

+     return tracker_ids

+ 

  def get_package_bugs(package):

      """

      Returns all open Bugs for a single package
@@ -292,11 +312,19 @@ 

  

      query = bzapi.url_to_query("https://bugzilla.redhat.com/buglist.cgi?bug_status=NEW&bug_status=__open__&"

                                  "classification=Fedora&product=Fedora&component=%s" % package)

-     query["include_fields"] = ["id", "summary", "version", "severity", "status", "last_change_time", "creation_time", "keywords"]

+     query["include_fields"] = ['blocks', 'comments', 'creation_time', 'id', 'keywords', 'last_change_time',

+                                'severity', 'status', 'summary', 'version']

      bugs = bzapi.query(query)

      if len(bugs) == 0:

          return []

+     ftbfs_trackers = set(CACHE.get('ftbfs_trackers'))

+     fti_trackers = set(CACHE.get('fti_trackers'))

      for bug in bugs:

+         for blocks in bug.blocks:

+             if blocks in ftbfs_trackers and "FTBFS" not in bug.keywords:

+                 bug.keywords.append("FTBFS")

+             if blocks in fti_trackers and "FTI" not in bug.keywords:

+                 bug.keywords.append("FTI")

          data.append({

              "title": bug.summary,

              "bug_id": bug.id,
@@ -306,6 +334,7 @@ 

              "reported": str(bug.creation_time),

              "release": bug.version,

              "keywords": bug.keywords,

+             "comments": (len(bug.comments) - 1), # Let's ignore the first comment that every bug has

              "url": "https://bugzilla.redhat.com/%s" % bug.id

          })

      return data
@@ -363,5 +392,69 @@ 

                  data[package].append(process_override(override))

      return data

  

+ def parse_koschei_data():

+     """

+     Prepares data from koschei for easier parsing in get_user_koschei_data()

+     Returns dict of lists containing dicts: {

+         "package_name": [

+             {"release": "fedora_release", "status": "koschei status", "url": "https://koschei.fedoraproject.org/package/..."}

+         ]

+     }

+     """

+     data = defaultdict(list)

+     koschei_resp = get_json(app.config['KOSCHEI_API_URL'])

+     for item in koschei_resp:

+         data[item["name"]].append({

+             "release": item["collection"],

+             "status": item["state"],

+             "url": "https://koschei.fedoraproject.org/package/%s?collection=%s" % (item["name"], item["collection"])

+             })

+     return data

+ 

+ def get_user_koschei_data(packages):

+     """

+     Filters out koschei data to contain information only about given pacakges list

+     """

+     koschei = CACHE.get("koschei_data")

+     data = {}

+     for package in packages:

+         data[package] = koschei.get(package, [])

+     return data

+ 

+ def get_health_check_data():

+     """

+     Retrieves json files containing data from fedora-health-check and combines them into dictionary with per-release keys

+     Returns dict {

+         "FXX":         "user_a"...: [package_a, package_b, ...]

+         "FXX-testing": "user_b"...: [package_a, package_b. ...]

+         "FXX":         "user_a"...: [package_b, package_c, ...] # Rawhide releases don't have -testing counterpart

+     }

+     """

+     data = {}

+     devel = str(CACHE.get('landing_page')["devel"])

+     stable = str(CACHE.get('landing_page')["stable"])

+     data[stable] = get_json("https://pagure.io/fedora-health-check/raw/master/f/data/report-%s.json" % stable)["affected"]

+     data[stable + "-testing"] = get_json("https://pagure.io/fedora-health-check/raw/master/f/data/report-%s-testing.json" % stable)["affected"]

+     data[devel] = get_json("https://pagure.io/fedora-health-check/raw/master/f/data/report-rawhide.json")["affected"]

+     return data

+ 

+ def get_health_check_user_data(user, packages):

+     """

+     Parses data from get_health_check_data and returns dict for single user and package list:

+     {

+         "package_a":  {"fails_to_install": True/False, "releases": [list of affected releases]

+         ...

+     }

+     """

+     fti_data = CACHE.get('health_check_data')

+     data = {}

+     for package in packages:

+         data[package] = {'fails_to_install': False, 'releases': []}

+         for release in fti_data:

+             if package in fti_data[release].get(user, []):

+                 data[package]['fails_to_install'] = True

+                 data[package]['releases'].append(release)

+     return data

+ 

  if __name__ == "__main__":

      import ipdb; ipdb.set_trace()

I'd try to also improve release numbers detection as a part of this PR.

Currently (if I remember correctly), during branched cycle, devel release returned from oraculum is actually branched number (which is correct for landing page usage).

I'll try to add one more release number - 'rawhide' to make monitoring FTBFS trackers more usable.

rebased onto c97ff2a

3 years ago

Took me a while to see what changed, since you are adding stuff in the middle of the list, could you make it alphabetically ordered? Absolute nitpick, thought.

1 new commit added

  • Nitpick
3 years ago

1 new commit added

  • FTI Trackers
3 years ago

simple if move_on: would be sufficient here

I'm not sure what the data and intent really are, but is it not possible to have this kind of situation?

bug.blocks = [1,2]
ftbfs_trackers = [1, 4]
fti_trackers = [2, 5]

My uneducated guess is that in this situation, you'd want to end up with both FTBFS and FTI in bug.keywords, but the code as-is ends up tagging the bug only with one of the FTBFS/FTI. Because that's what the code does - as soon as the first of these two is hit, the whole bug.blocks processing stops.

rebased onto 7826a62

3 years ago

Yep, thanks @jskladan , I've overlooked it.

I've cleaned up the code a little and added comment counting for bugs.

rebased onto 8cb33a1

3 years ago

for both of these:
1) ... = set(CACHE.get(...))
2) ???
3) profit

1 new commit added

  • Use sets instead of lists for ft(bs/i) tacker ids
3 years ago

1 new commit added

  • [HEAVY_WIP] Koschei
3 years ago

1 new commit added

  • Koschei: Wire up caching
3 years ago

1 new commit added

  • Bugs: comment_count > comments
3 years ago

1 new commit added

  • Docs
3 years ago

1 new commit added

  • Simplify koschei
3 years ago

1 new commit added

  • Spaces
3 years ago

1 new commit added

  • Add keys for packages not in koschei
3 years ago

How about collections.defaultdict(list)? That would make the code below much simpler.

If you decide against defaultdict, then please change the code below to something like this pseudocode:

   record = {"release": ..., "status": ..., "url": ...}
   if item["name"] in data:
        data[item["name"]].append(record)
   else:
       data[item["name"]] = [record]

this whole thing is IMO way overcomplicated for what is does, IIUIC, this does the same, in a better way:

koschei = CACHE.get("koschei_data")
data = {}
for p in packages:
    data[p] = koschei_data.get(p, [])

1 new commit added

  • Cleanup
3 years ago

13 new commits added

  • Cleanup
  • Add keys for packages not in koschei
  • Spaces
  • Simplify koschei
  • Docs
  • Bugs: comment_count > comments
  • Koschei: Wire up caching
  • [HEAVY_WIP] Koschei
  • Use sets instead of lists for ft(bs/i) tacker ids
  • Cleanup and comment counts in bugs
  • FTI Trackers
  • Nitpick
  • Append FTBFS to bug keywords where applicable
3 years ago

1 new commit added

  • Wire up fedora-health-check
3 years ago

1 new commit added

  • Docs
3 years ago

This could be rewritten to something like: if package in fti_data[release].get(user, []) to get rid of that one "unnecessary" item in list-hidden-for-cycle. On top of that, I personaly find it a bit more readable, but that's just personal preference. It could also be rewritten to if fti_data[release].get(user) and package in fti_data[release][user] to get the same effect, but this IMO reads way worse than any of the other two versions. I'm just mentioning it so you can dive into the whole "how the hell does this sh*t work" and "how are the if-clauses really executed".

Apart of the nitpick, LGTM, thanks!

1 new commit added

  • Nitpick
3 years ago

rebased onto d5f2e7d

3 years ago

Pull-Request has been merged by frantisekz

3 years ago