From 1144f6ee0961df7597d058e2c9901d53af266284 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Jul 19 2020 20:48:56 +0000 Subject: frontend, python, cli: more effective query of packages with their latest builds Fix #757 Instead obtaining a list of packages in copr-cli and then sending another request per package to obtain their latest or latest succeeded build, let's just send a `builds` dictionary within every package. This addition is not required by every user, so let's make it opt-in. When any build are requested, then the `builds` field will contain `None` values, such as [ { "name": "foo", ... "builds": { "latest": null, "latest_succeeded": null }, } ] When a request from client contains either of "with_latest_build": True, "with_latest_succeeded_build": True, the particular build information is going to be filled with a build dictionary, that has the exact same structure as a build queried through `BuildProxy`. Lastly, copr-cli provides also `--with-all-builds` for obtaining a package with all it's builds. This option is not (yet) reworked and added to the API, so this will still send an additional request per package. The list of the package builds will be then appended to the `builds` dictionary and accessible by e.g. `package["builds"]["all"]`. Some measurements at the end. The speed of both $ copr-cli list-packages foo/bar $ copr-cli list-packages foo/bar --with-all-builds remains unchanged. However, these $ copr-cli list-packages foo/bar --with-latest-build $ copr-cli list-packages foo/bar --with-latest-succeeded are now finished within 30-40s for the most gigantic projects with 15k+ packages. That is a big improvement and huge drop from previous ~12 minutes. Speed of those commands is now comparable with rendering the packages page in a browser, which takes about the same time (~40s). --- diff --git a/cli/copr_cli/main.py b/cli/copr_cli/main.py index f549ee8..2617c5a 100644 --- a/cli/copr_cli/main.py +++ b/cli/copr_cli/main.py @@ -624,7 +624,9 @@ class Commands(object): def action_list_packages(self, args): ownername, projectname = self.parse_name(args.copr) - packages = self.client.package_proxy.get_list(ownername=ownername, projectname=projectname) + packages = self.client.package_proxy.get_list(ownername=ownername, projectname=projectname, + with_latest_build=args.with_latest_build, + with_latest_succeeded_build=args.with_latest_succeeded_build) packages_with_builds = [self._package_with_builds(p, args) for p in packages] print(json_dumps(packages_with_builds)) @@ -643,19 +645,10 @@ class Commands(object): def _package_with_builds(self, package, args): ownername, projectname = self.parse_name(args.copr) kwargs = {"ownername": ownername, "projectname": projectname, "packagename": package.name} - pagination = {"limit": 1, "order": "id", "order_type": "DESC"} - - if args.with_latest_build: - builds = self.client.build_proxy.get_list(pagination=pagination, **kwargs) - package["latest_build"] = builds[0] if builds else None - - if args.with_latest_succeeded_build: - builds = self.client.build_proxy.get_list(status="succeeded", pagination=pagination, **kwargs) - package["latest_succeeded_build"] = builds[0] if builds else None if args.with_all_builds: builds = self.client.build_proxy.get_list(**kwargs) - package["builds"] = builds + package["builds"]["all"] = builds return package diff --git a/frontend/coprs_frontend/coprs/logic/packages_logic.py b/frontend/coprs_frontend/coprs/logic/packages_logic.py index a49692d..5abfa64 100644 --- a/frontend/coprs_frontend/coprs/logic/packages_logic.py +++ b/frontend/coprs_frontend/coprs/logic/packages_logic.py @@ -35,7 +35,7 @@ class PackagesLogic(object): .filter(models.Package.copr_id == copr_id)) @classmethod - def get_packages_with_latest_builds_for_dir(cls, copr_dir_id): + def get_packages_with_latest_builds_for_dir(cls, copr_dir_id, small_build=True): packages = (models.Package.query.filter_by(copr_dir_id=copr_dir_id) .order_by(models.Package.name).all()) pkg_ids = [package.id for package in packages] @@ -58,15 +58,19 @@ class PackagesLogic(object): if not build.package_id: continue - small_build_object = SmallBuild() - for param in ['state', 'status', 'pkg_version', - 'submitted_on']: - # we don't want to keep all the attributes here in memory, and - # also we don't need any further info about assigned - # build_chroot(s). So we only pick the info we need, and throw - # the expensive objects away. - setattr(small_build_object, param, getattr(build, param)) - packages_map[build.package_id].latest_build = small_build_object + if small_build: + small_build_object = SmallBuild() + for param in ['state', 'status', 'pkg_version', + 'submitted_on']: + # we don't want to keep all the attributes here in memory, and + # also we don't need any further info about assigned + # build_chroot(s). So we only pick the info we need, and throw + # the expensive objects away. + setattr(small_build_object, param, getattr(build, param)) + packages_map[build.package_id].latest_build = small_build_object + else: + packages_map[build.package_id].latest_build = build + return packages diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py index e43f985..e65f6db 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py @@ -165,6 +165,34 @@ class Paginator(object): def to_dict(self): return [x.to_dict() for x in self.get()] + +class ListPaginator(Paginator): + """ + The normal `Paginator` class works with a SQLAlchemy query object and + therefore can do limits and ordering on database level, which is ideal. + However, in some special cases, we already have a list of objects fetched + from database and need to adjust it based on user pagination preferences, + hence this special case of `Paginator` class. + + It isn't efficient, it isn't pretty. Please use `Paginator` if you can. + """ + def get(self): + objects = self.query + reverse = self.order_type != "ASC" + + if not hasattr(self.model, self.order): + raise CoprHttpException("Can order by: {}".format(self.order)) + + if self.order: + objects.sort(key=lambda x: getattr(x, self.order), reverse=reverse) + + limit = None + if self.limit: + limit = self.offset + self.limit + + return objects[self.offset : limit] + + def editable_copr(f): @wraps(f) def wrapper(ownername, projectname, **kwargs): diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_packages.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_packages.py index 84fa307..c84ece8 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_packages.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_packages.py @@ -1,5 +1,5 @@ import flask -from . import query_params, pagination, get_copr, Paginator, GET, POST, PUT, DELETE +from . import query_params, pagination, get_copr, ListPaginator, GET, POST, PUT, DELETE from .json2form import get_form_compatible_data, get_input, without_empty_fields from coprs.exceptions import (ObjectNotFound, BadRequest) from coprs.views.misc import api_login_required @@ -14,10 +14,21 @@ from coprs.views.apiv3_ns.apiv3_builds import to_dict as build_to_dict from coprs.views.api_ns.api_general import process_package_add_or_edit -def to_dict(package): +def to_dict(package, with_latest_build=False, with_latest_succeeded_build=False): source_dict = package.source_json_dict if "srpm_build_method" in source_dict: source_dict["source_build_method"] = source_dict.pop("srpm_build_method") + + latest = None + if with_latest_build: + latest = package.last_build() + latest = build_to_dict(latest) if latest else None + + latest_succeeded = None + if with_latest_succeeded_build: + latest_succeeded = package.last_build(successful=True) + latest_succeeded = build_to_dict(latest_succeeded) if latest_succeeded else None + return { "id": package.id, "name": package.name, @@ -26,6 +37,10 @@ def to_dict(package): "source_type": package.source_type_text, "source_dict": source_dict, "auto_rebuild": package.webhook_rebuild, + "builds": { + "latest": latest, + "latest_succeeded": latest_succeeded, + } } @@ -60,10 +75,16 @@ def get_package(ownername, projectname, packagename): @apiv3_ns.route("/package/list/", methods=GET) @pagination() @query_params() -def get_package_list(ownername, projectname, **kwargs): +def get_package_list(ownername, projectname, with_latest_build=False, + with_latest_succeeded_build=False, **kwargs): + with_latest_build = with_latest_build != "False" + with_latest_succeeded_build = with_latest_succeeded_build != "False" + copr = get_copr(ownername, projectname) - paginator = Paginator(PackagesLogic.get_all(copr.main_dir.id), models.Package, **kwargs) - packages = paginator.map(to_dict) + packages = PackagesLogic.get_packages_with_latest_builds_for_dir(copr.main_dir.id, small_build=False) + paginator = ListPaginator(packages, models.Package, **kwargs) + + packages = paginator.map(lambda x: to_dict(x, with_latest_build, with_latest_succeeded_build)) return flask.jsonify(items=packages, meta=paginator.meta) diff --git a/python/copr/v3/proxies/package.py b/python/copr/v3/proxies/package.py index dfb9b7e..97a5987 100644 --- a/python/copr/v3/proxies/package.py +++ b/python/copr/v3/proxies/package.py @@ -25,19 +25,27 @@ class PackageProxy(BaseProxy): response = request.send() return munchify(response) - def get_list(self, ownername, projectname, pagination=None): + def get_list(self, ownername, projectname, pagination=None, + with_latest_build=False, with_latest_succeeded_build=False): """ Return a list of packages :param str ownername: :param str projectname: :param pagination: + :param bool with_latest_build: The result will contain "builds" dictionary with the latest + submitted build of this particular package within the project + :param bool with_latest_succeeded_build: The result will contain "builds" dictionary with + the latest successful build of this particular + package within the project. :return: Munch """ endpoint = "/package/list" params = { "ownername": ownername, "projectname": projectname, + "with_latest_build": with_latest_build, + "with_latest_succeeded_build": with_latest_succeeded_build, } params.update(pagination or {})