fedora-qa / oraculum

Clone
Source Code
GIT

#191 Enforce separate groups from users
Merged 2 years ago by frantisekz. Opened 2 years ago by frantisekz.
apiv2_dashboard_separate_groups  into  master

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

  

  import os
 

  import sys
 

- import tempfile
 

  

  from collections import defaultdict

file modified
+41 -17 
@@ -22,6 +22,8 @@ 
 

  from flask import jsonify, request
 

  from flask_login import current_user
 

  

+ import re
 

+ 

  from oraculum import app, CACHE
 

  from oraculum.utils import bodhi, bugzilla, cache_utils, celery_utils, calendars, dashboard_helpers, health_check, koschei, orphans, pagure, versions, retrace_server, watchdog_utils
 

  
@@ -126,25 +128,19 @@ 
 

      }
 

  

  @app.route('/api/v2/packager_dashboard')
 

- def route_dashboard_data(users="", pkgs=""):
 

+ def route_dashboard_data(users="", groups="", pkgs=""):
 

  

      users = request.args["users"].split(",") if "users" in request.args else []
 

+     groups = request.args["groups"].split(",") if "groups" in request.args else []
 

      pkgs = request.args["packages"].split(",") if "packages" in request.args else []
 

  

-     if not any([users, pkgs]):
 

+     if not any([users, groups, pkgs]):
 

          return jsonify({
 

              "packages": None,
 

              "status": 400,
 

-             "error": "The correct format to query this endpoint is /api/v2/packager_dashboard?users=[users]&packages=[packages]"
 

+             "error": "The correct format to query this endpoint is /api/v2/packager_dashboard?users=[users]&groups=[groups]&packages=[packages]"
 

          }), 400
 

  

-     package_owners_raw = CACHE.get('packages_owners_json')
 

-     pagure_groups_raw = CACHE.get('pagure_groups')
 

- 

-     # Run some checks on requested items - if naming is correct and if explicit packages exist
 

-     users = {dashboard_helpers.clean_fas_username(user) for user in users}
 

-     pkgs = {dashboard_helpers.clean_fas_username(package) for package in pkgs}.intersection(package_owners_raw["rpms"].keys())
 

- 

      package_data_dict = {
 

          'calendars': [],
 

          'orphans': [],
 
@@ -167,13 +163,23 @@ 
 

          orphan_data = v2_handle_orphan(users, pkgs, {**package_data_dict})
 

          return jsonify(orphan_data), orphan_data["status"]
 

  

+     package_owners_raw = CACHE.get('packages_owners_json')
 

+     pagure_groups_raw = CACHE.get('pagure_groups')
 

+ 

+     # Filter out what violates naming rules and doesn't make sense to process further
 

+     allowed_groups = [re.compile(a) for a in app.config["ALLOWED_PACKAGER_GROUPS"]]
 

+     users = {dashboard_helpers.clean_fas_username(user) for user in users
 

+             if not any(regex.match(user) for regex in allowed_groups)}
 

+     groups = {group for group in groups if any(regex.match(group) for regex in allowed_groups)}
 

+     pkgs = {dashboard_helpers.clean_fas_username(package) for package in pkgs}.intersection(package_owners_raw["rpms"].keys())
 

+ 

      for user in users:
 

          user_packages = pagure.get_packages(user, package_owners_raw, pagure_groups_raw)
 

  

          for package in user_packages["primary"]:
 

              if package in packages:
 

-                 # If we already have a package, just append current user to maintainers
 

-                 packages[package]["maintainers"]["users"].append(user)
 

+                 if user not in packages[package]["maintainers"]["users"]:
 

+                     packages[package]["maintainers"]["users"].append(user)
 

                  continue
 

  

              packages[package] = {
 
@@ -183,14 +189,14 @@ 
 

                  "refresh_times": {**refresh_times}
 

              }
 

  

-             # Remove package from explicit packages if we have it here
 

+             # Don't duplicitly re-process the package later in explicitly requested packages pass
 

              pkgs.discard(package)
 

  

          for group in user_packages["group"]:
 

              for package in user_packages["group"][group]:
 

                  if package in packages:
 

-                     # If we already have a package, just append current group to maintainers
 

-                     packages[package]["maintainers"]["groups"].append(group)
 

+                     if group not in packages[package]["maintainers"]["groups"]:
 

+                         packages[package]["maintainers"]["groups"].append(group)
 

                      continue
 

  

                  packages[package] = {
 
@@ -200,9 +206,27 @@ 
 

                      "refresh_times": {**refresh_times}
 

                  }
 

  

-                 # Remove package from explicit packages if we have it here
 

+                 # Don't duplicitly re-process the package later in explicitly requested packages pass
 

                  pkgs.discard(package)
 

  

+     for group in groups:
 

+         group_packages = pagure.get_packages(group, package_owners_raw, pagure_groups_raw)
 

+         for package in group_packages["primary"]:
 

+             if package in packages:
 

+                 if group not in packages[package]["maintainers"]["groups"]:
 

+                     packages[package]["maintainers"]["groups"].append(group)
 

+                 continue
 

+ 

+             packages[package] = {
 

+                 "package": package,
 

+                 "maintainers": {"users": [], "groups": [group]},
 

+                 "data": {**package_data_dict},
 

+                 "refresh_times": {**refresh_times}
 

+             }
 

+ 

+             # Don't duplicitly re-process the package later in explicitly requested packages pass
 

+             pkgs.discard(package)
 

+ 

      for package in pkgs:
 

          packages[package] = {
 

              "package": package,
 
@@ -213,7 +237,7 @@ 
 

  

      flat_pkgs_list = packages.keys()
 

      dashboard_helpers.update_packages_access_time(flat_pkgs_list)
 

-     dashboard_helpers.update_users_access_time(users)
 

+     dashboard_helpers.update_users_access_time(set().union(users, groups))
 

  

      generic_data = dashboard_data_generic(flat_pkgs_list)
 

      refresh_times_dict = {key: generic_data["last_synced"] for key in package_data_dict}

no initial comment

Build succeeded.

2 new commits added

  • [DRAFT] Enforce separate groups from users
  • Pre-compile allowed groups on init
2 years ago

2 new commits added

  • [DRAFT] Enforce separate groups from users
  • Pre-compile allowed groups on init
2 years ago

2 new commits added

  • [DRAFT] Enforce separate groups from users
  • Pre-compile allowed groups on init
2 years ago

2 new commits added

  • Enforce separate groups from users
  • Pre-compile allowed groups on init
2 years ago

Build succeeded.

1 new commit added

  • Dashboard: optimize orphan user
2 years ago

Build succeeded.

3 new commits added

  • Dashboard: optimize orphan user
  • Enforce separate groups from users
  • Pre-compile allowed groups on init
2 years ago

Why filledlist or emptyset? Does having an empty set instead of an empty list has some huge advantage here?

Build succeeded.

I'd like to see this variable named... better? Maybe to reflect it is a list of regexps? Also the comment before it should describe the intent of the code, this is next to useless (yet funny) 17 characters (also, ellipsis is three dots, not four ;))

Why filledlist or emptyset? Does having an empty set instead of an empty list has some huge advantage here?

It would make sense to, once again, describe the intent here. It's kinda obvious this is "running checks", describe why, not what.

This is stretching the limit of what an one-liner should look like. Please reformat to multiple lines for better readability.

once again, the comment (if any) should not describe "what" the code does (especially since it describes the code word-for-word), but why it's done.

Do I even need to comment on this? You guessed it, describe "why" not "what" :)

Also, looking at the rest of the code, I'm fairly certain (can be mistaken) that this won't work for anything other than the specific value you have assigned in the config.py

You should try changing the value in an actual config file, and in the env-var (Openshift specific IIRC), and observe the results, but AFAICT this won't work as you (probably) intended.

rebased onto 0ac6366
2 years ago

Build succeeded.

I'd like this more, if the whole for user in users was on this line :)

rebased onto 711d65c
2 years ago

Build succeeded.

Pull-Request has been merged by frantisekz
2 years ago
Metadata
None
No Tags
Changes Summary 2
+0 -1
file changed
oraculum/config.py
+41 -17
file changed
oraculum/controllers/packager_dashboard.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.