#1908 Speedup frontend for large projects
Merged 3 years ago by praiskup. Opened 3 years ago by praiskup.

@@ -0,0 +1,21 @@

+ """

+ Provide index for Build: id.desc() + copr_dir_id

+ 

+ Revision ID: d5990bd4aa46

+ Revises: 45d0515bdc3f

+ Create Date: 2021-08-31 14:43:28.534127

+ """

+ 

+ import sqlalchemy as sa

+ from alembic import op

+ 

+ 

+ revision = 'd5990bd4aa46'

+ down_revision = '45d0515bdc3f'

+ 

+ def upgrade():

+     op.create_index('build_id_desc_per_copr_dir', 'build',

+                     [sa.text('id DESC'), 'copr_dir_id'], unique=False)

+ 

+ def downgrade():

+     op.drop_index('build_id_desc_per_copr_dir', table_name='build')

@@ -112,6 +112,7 @@

  from coprs.views.coprs_ns import coprs_general

  from coprs.views.coprs_ns import coprs_chroots

  from coprs.views.coprs_ns import coprs_packages

+ from coprs.views.coprs_ns import pagination_redirect

  from coprs.views import backend_ns

  from coprs.views.backend_ns import backend_general

  from coprs.views import misc
@@ -141,7 +142,7 @@

      NonAdminCannotDisableAutoPrunning,

  )

  from coprs.error_handlers import get_error_handler

- from .context_processors import include_banner, inject_fedmenu, counter_processor

+ import coprs.context_processors

  

  setup_log()

  

@@ -1,7 +1,10 @@

  import os

- from . import app

  import flask

  

+ from coprs import app

+ from coprs.helpers import current_url

+ 

+ 

  BANNER_LOCATION = "/var/lib/copr/banner-include.html"

  

  
@@ -82,3 +85,9 @@

          return str(flask.g.counters[name])

  

      return dict(counter=counter)

+ 

+ 

+ @app.context_processor

+ def current_url_processor():

+     """ Provide 'current_url()' method in templates """

+     return dict(current_url=current_url)

@@ -20,6 +20,8 @@

  from sqlalchemy.engine.default import DefaultDialect

  from sqlalchemy.sql.sqltypes import String, DateTime, NullType

  

+ from werkzeug.urls import url_encode

+ 

  from copr_common.enums import EnumType

  # TODO: don't import BuildSourceEnum from helpers, use copr_common.enum instead

  from copr_common.enums import BuildSourceEnum # pylint: disable=unused-import
@@ -700,3 +702,16 @@

          setattr(new_instance, attr, rel)

  

      return new_instance

+ 

+ 

+ def current_url(**kwargs):

+     """

+     Generate the same url as is currently processed, but define (or replace) the

+     arguments in kwargs.

+     """

+     new_args = {}

+     new_args.update(flask.request.args)

+     new_args.update(kwargs)

+     if not new_args:

+         return flask.request.path

+     return '{}?{}'.format(flask.request.path, url_encode(new_args))

@@ -1400,40 +1400,74 @@

  

  class BuildsMonitorLogic(object):

      @classmethod

-     def get_monitor_data(cls, copr):

-         query = """

- 	SELECT

- 	  package.id as package_id,

- 	  package.name AS package_name,

- 	  build.id AS build_id,

- 	  build_chroot.status AS build_chroot_status,

- 	  build.pkg_version AS build_pkg_version,

- 	  mock_chroot.id AS mock_chroot_id,

-           mock_chroot.os_release AS mock_chroot_os_release,

-           mock_chroot.os_version AS mock_chroot_os_version,

-           mock_chroot.arch AS mock_chroot_arch

- 	FROM package

- 	JOIN (SELECT

- 	  MAX(build.id) AS max_build_id_for_chroot,

- 	  build.package_id AS package_id,

- 	  build_chroot.mock_chroot_id AS mock_chroot_id

- 	FROM build

- 	JOIN build_chroot

- 	  ON build.id = build_chroot.build_id

- 	WHERE build.copr_id = {copr_id}

- 	AND build_chroot.status != 2

- 	GROUP BY build.package_id,

- 		 build_chroot.mock_chroot_id) AS max_build_ids_for_a_chroot

- 	  ON package.id = max_build_ids_for_a_chroot.package_id

- 	JOIN build

- 	  ON build.id = max_build_ids_for_a_chroot.max_build_id_for_chroot

- 	JOIN build_chroot

- 	  ON build_chroot.mock_chroot_id = max_build_ids_for_a_chroot.mock_chroot_id

- 	  AND build_chroot.build_id = max_build_ids_for_a_chroot.max_build_id_for_chroot

- 	JOIN mock_chroot

- 	  ON mock_chroot.id = max_build_ids_for_a_chroot.mock_chroot_id

- 	JOIN copr_dir ON build.copr_dir_id=copr_dir.id WHERE copr_dir.main IS TRUE

- 	ORDER BY package.name ASC, package.id ASC, mock_chroot.os_release ASC, mock_chroot.os_version ASC, mock_chroot.arch ASC

- 	""".format(copr_id=copr.id)

-         rows = db.session.execute(query)

-         return rows

+     def get_monitor_data(cls, copr, per_page=50, page=1,

+                          paginate_if_more_than=1000):

+         """

+         Return Package objects with corresponding latest BuildChroots in active

+         CoprChroots in given COPR.  We try to hold all packages on one page,

+         though if there are more than PAGINATE_IF_MORE_THAN packages, we return

+         only a slice of the results (in this case PAGE is used to return an

+         appropriate slice, and PER_PAGE to control how many pieces are returned

+         in one slice).

+         We return only the pagination object:

+             pagination.items => list of returned packages

+             pagination.serverside_pagination => if the pagination is in effect

+             pagination.item[N].latest_build_chroots => list of the latest

+                 BuildChroots (can span multiple Builds!) for the given package

+         """

+ 

+         packages_query = PackagesLogic.get_all_ordered(copr.main_dir.id)

+         packages_count = packages_query.count()

+ 

+         if packages_count > paginate_if_more_than:

+             pagination = packages_query.paginate(per_page=per_page,

+                                                  page=page)

+             pagination.serverside_pagination = True

+         else:

+             pagination = packages_query.paginate(per_page=paginate_if_more_than,

+                                                  page=1)

+             pagination.serverside_pagination = False

+ 

+         pkg_ids = [package.id for package in pagination.items]

+         mock_chroot_ids = [mch.id for mch in copr.active_chroots]

+ 

+         builds_ids = (

+             models.Build.query.join(models.BuildChroot)

+                   .filter(models.Build.package_id.in_(pkg_ids))

+                   .filter(models.BuildChroot.mock_chroot_id.in_(mock_chroot_ids))

+                   .with_entities(

+                       models.Build.package_id.label('package_id'),

+                       func.max(models.Build.id).label('build_id').label("build_id"),

+                       models.BuildChroot.mock_chroot_id,

+                   )

+                   .group_by(

+                       models.Build.package_id,

+                       models.BuildChroot.mock_chroot_id,

+                   )

+                   .subquery()

+         )

+ 

+         query = (models.BuildChroot.query

+             .join(

+                 builds_ids,

+                 and_(builds_ids.c.build_id == models.BuildChroot.build_id,

+                      builds_ids.c.mock_chroot_id == models.BuildChroot.mock_chroot_id))

+             .add_columns(

+                 builds_ids.c.package_id.label("package_id"),

+             )

+         )

+ 

+         mapper = {}

+         for package in pagination.items:

+             mapper[package.id] = {}

+ 

+         for build_chroot, package_id in query:

+             mapper[package_id][build_chroot.mock_chroot.name] = build_chroot

+ 

+         for package in pagination.items:

+             package.latest_build_chroots = []

+             for chroot in copr.active_chroots_sorted:

+                 package.latest_build_chroots.append(

+                     mapper[package.id].get(chroot.name))

+ 

+         return pagination

@@ -619,10 +619,11 @@

          """

          subquery = (

              db.session.query(

-                 func.max(models.Build.submitted_on)

+                 models.Build.submitted_on

              )

              .filter(models.Build.copr_dir_id==models.CoprDir.id)

-             .group_by(models.Build.copr_dir_id)

+             .order_by(desc(models.Build.id))

+             .limit(1)

              .label("latest_build_submitted_on")

          )

          return (

@@ -26,18 +26,42 @@

  

      @classmethod

      def get_all(cls, copr_dir_id):

+         """

+         Get all packages assigned to 'copr_dir_id'

+         """

          return (models.Package.query

                  .filter(models.Package.copr_dir_id == copr_dir_id))

  

      @classmethod

+     def get_all_ordered(cls, copr_dir_id):

+         """

+         Get all packages in 'copr_dir_id' ordered by package name

+         """

+         return cls.get_all(copr_dir_id).order_by(models.Package.name)

+ 

+     @classmethod

      def get_all_in_copr(cls, copr_id):

          return (models.Package.query

                  .filter(models.Package.copr_id == copr_id))

  

      @classmethod

-     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())

+     def get_packages_with_latest_builds_for_dir(

+             cls, copr_dir_id, small_build=True, packages=None):

+         """

+         Obtain the list of package objects for given copr_dir_id, with the

+         latest build assigned.

+         Parameters:

+ 

+         :param copr_dir_id: CoprDir ID (int)

+         :param small_build: Don't assign full Build objects, but only a limited

+             objects with necessary info.

+         :param packages: Don't query the list of Package objects from DB, but

+             use the given 'packages' array.

+         :return: array of Package objects, with assigned latest Build object

+         """

+         if not packages:

+             packages = cls.get_all_ordered(copr_dir_id).all()

+ 

          pkg_ids = [package.id for package in packages]

          builds_ids = models.Build.query \

              .filter(models.Build.package_id.in_(pkg_ids)) \

@@ -944,13 +944,6 @@

      SCM_COMMIT = 'commit'

      SCM_PULL_REQUEST = 'pull-request'

  

-     __table_args__ = (db.Index('build_canceled', "canceled"),

-                       db.Index('build_order', "is_background", "id"),

-                       db.Index('build_filter', "source_type", "canceled"),

-                       db.Index('build_canceled_is_background_source_status_id_idx', 'canceled', "is_background", "source_status", "id"),

-                       db.Index('build_copr_id_package_id', "copr_id", "package_id")

-                      )

- 

      def __init__(self, *args, **kwargs):

          if kwargs.get('source_type') == helpers.BuildSourceEnum("custom"):

              source_dict = json.loads(kwargs['source_json'])
@@ -1040,6 +1033,16 @@

      # the info that the build was resubmitted

      resubmitted_from_id = db.Column(db.Integer)

  

+     __table_args__ = (

+         db.Index('build_canceled', "canceled"),

+         db.Index('build_order', "is_background", "id"),

+         db.Index('build_filter', "source_type", "canceled"),

+         db.Index('build_canceled_is_background_source_status_id_idx',

+                  'canceled', "is_background", "source_status", "id"),

+         db.Index('build_copr_id_package_id', "copr_id", "package_id"),

+         db.Index("build_id_desc_per_copr_dir", id.desc(), "copr_dir_id"),

+     )

+ 

      _cached_status = None

      _cached_status_set = None

  

@@ -475,8 +475,8 @@

  {% endmacro %}

  

  

- {% macro render_monitor_table(copr, monitor, oses, archs, simple=True) %}

- <table class="table table-striped table-bordered">

+ {% macro render_monitor_table(copr, pagination, oses, archs, simple=True) %}

+ <table class="table table-striped table-bordered dataTable">

    <thead>

      <tr>

        <th rowspan="2">Package</th>
@@ -492,56 +492,41 @@

    </thead>

    <tbody>

      <tr>

-     {# The following code is optimized to pass (potentially large) monitor data just once. #}

-     {# It expects results to be sorted by package.name and then mock_chroot.name. #}

+     {# This table should be sorted by package name #}

      {% set current_row = [None] %}

      {% set copr_active_chroots_sorted = copr.active_chroots_sorted %}

      {% set copr_active_chroots_sorted_length = copr_active_chroots_sorted|length %}

      {#% set copr_active_chroots_sorted_index = copr_active_chroots_sorted_length %#} {# we need to set this var from an inner scope, which is unsupported, hence the following list-as-a-counter hacks: #}

      {% set copr_active_chroots_sorted_index = [1]*copr_active_chroots_sorted_length %}

  

-     {% for row in monitor %}

-       {% if row.package_name != current_row[0] %}

-         {% for _ in range(copr_active_chroots_sorted_index|length, copr_active_chroots_sorted_length) %} {# "do" tag would help but just a tiny bit (so no enabling) #}

-           <td>-</td>

-         {% endfor %}

-         </tr><tr>

+     </tr>

  

+     {% for package in pagination.items %}

+     <tr>

          <td style="white-space:nowrap">

-           <b><a href="{{ copr_url('coprs_ns.copr_package', copr, package_name=row.package_name) }}">

-             {{ row.package_name }}

+           <b><a href="{{ copr_url('coprs_ns.copr_package', copr, package_name=package.name) }}">

+             {{ package.name }}

            </a></b>

          </td>

-         {% set _ = current_row.pop() %}

-         {% set _ = current_row.append(row.package_name) %}

-         {#% set copr_active_chroots_sorted_index = 0 %#}

-         {% for _ in range(copr_active_chroots_sorted_index|length) %}{% if copr_active_chroots_sorted_index.remove(1) %}{% endif %}{% endfor %}

-       {% endif %}

- 

-       {% for index in range(copr_active_chroots_sorted_index|length, copr_active_chroots_sorted_length) %}

-         {% if row.mock_chroot_id == copr_active_chroots_sorted[index].id %}

-           {% for _ in range(copr_active_chroots_sorted_index|length, index) %}

-           <td>-</td>

-           {% endfor %}

-           <td>

-             <a href="{{ copr_url("coprs_ns.copr_build", copr, build_id=row.build_id) }}">

-               {% if simple %}

-                 {{ build_state_text(row.build_chroot_status|state_from_num) }}

-               {% else %}

-                 <small> {{ row.build_id }} </small> <br>

-                 {{ build_state_text(row.build_chroot_status|state_from_num) }}<br>

-                 <small class="text-muted"> {{ row.build_pkg_version }} </small>

-               {% endif %}

-             </a>

-           </td>

-           {#% set copr_active_chroots_sorted_index = index + 1 %#}

-           {% for _ in range(copr_active_chroots_sorted_index|length, index+1) %}

-             {% if copr_active_chroots_sorted_index.append(1) %}{% endif %}

-           {% endfor %}

-           {% set index = copr_active_chroots_sorted_length %}

-         {% endif %}

-       {% endfor %}

  

+         {% for buildchroot in package.latest_build_chroots %}

+         <td>

+             {% if buildchroot %}

+                 <a href="{{ copr_url("coprs_ns.copr_build", copr, build_id=buildchroot.build_id) }}">

+                 {% if simple %}

+                     {{ build_state_text(buildchroot.status|state_from_num) }}

+                 {% else %}

+                     <small> {{ buildchroot.build_id }} </small> <br>

+                     {{ build_state_text(buildchroot.status|state_from_num) }}<br>

+                     <small class="text-muted"> {{ buildchroot.build.pkg_version }} </small>

+                 {% endif %}

+                 </a>

+             {% else %}

+                 -

+             {% endif %}

+         </td>

+         {% endfor %}

+     </tr>

      {% endfor %}

  

      {% for _ in range(copr_active_chroots_sorted_index|length, copr_active_chroots_sorted_length) %} {# "do" tag would help but just a tiny bit (so no enabling) #}
@@ -550,6 +535,10 @@

      </tr>

    </tbody>

  </table>

+ 

+ {% if pagination.serverside_pagination %}

+ {{ pagination_form(pagination) }}

+ {% endif %}

  {% endmacro %}

  

  {% macro render_bootstrap_options(form, build=False) %}
@@ -866,3 +855,39 @@

      </td>

  </tr>

  {% endmacro %}

+ 

+ 

+ {% macro pagination_form(pagination) %}

+ <form class="content-view-pf-pagination table-view-pf-pagination clearfix"

+       id="pagination" method="post" action="{{ current_url() }}">

+   <div class="form-group">

+     <span>

+       <span class="pagination-pf-items-current">

+           {{ pagination.per_page*(pagination.page -1) + 1}} -

+           {{ pagination.per_page*(pagination.page) }}

+       </span>

+       of

+       <span class="pagination-pf-items-total">{{ pagination.total }}</span></span>

+     <ul class="pagination pagination-pf-back">

+       <li {% if not pagination.has_prev %}class="disabled"{% endif %}><a href="{{ current_url(page=1) }}" title="First Page"><span class="i fa fa-angle-double-left"></span></a></li>

+       <li {% if not pagination.has_prev %}class="disabled"{% endif %}><a href="{{ current_url(page=pagination.page - 1) }}"  title="Previous Page"><span class="i fa fa-angle-left"></span></a></li>

+     </ul>

+     <label for="pagination1-page" class="sr-only">Current Page</label>

+     <input class="pagination-pf-page" name="go_to_page"  type="text" value="{{ pagination.page }}" id="go_to_page" style="width: 3.5em;" />

+     <span>of <span class="pagination-pf-pages">{{ pagination.pages }}</span></span>

+     <ul class="pagination pagination-pf-forward">

+         <li {% if not pagination.has_next %}class="disabled"{% endif %}><a href="{{ current_url(page=pagination.page + 1) }}" title="Next Page"><span class="i fa fa-angle-right"></span></a></li>

+         <li {% if not pagination.has_next %}class="disabled"{% endif %}><a href="{{ current_url(page=pagination.pages) }}" title="Last Page"><span class="i fa fa-angle-double-right"></span></a></li>

+     </ul>

+   </div>

+ </form>

+ 

+ {% endmacro %}

+ 

+ {% macro serverside_pagination_warning() %}

+ <p> Warning!

+     This is a large project with many packages and/or builds - we can not hold all

+     the requested data on one page (as we usually do), please use the pagination

+     buttons below the table.

+ </p>

+ {% endmacro %}

@@ -1,13 +1,16 @@

  {% from "coprs/detail/_builds_forms.html" import copr_build_cancel_form, copr_build_repeat_form, copr_build_delete_form %}

  {% from "coprs/detail/_build_states.html" import build_states %}

  {% from "_helpers.html" import build_href_from_sql, build_state, initialize_datatables, copr_url %}

+ {% from "_helpers.html" import pagination_form with context %}

  

  

- {% macro builds_table(builds, print_possible_states=True) %}

+ {% macro builds_table(builds, print_possible_states=True, serverside_pagination=None) %}

    {% for build in builds %}

      {% if loop.first %}

+     {% if not serverside_pagination %}

      <noscript><p>WARNING!! This page is using JavaScript to filter, sort and delete builds from the table.</p></noscript>

-     <table class="datatable table table-striped table-bordered">

+     {% endif %}

+     <table class="datatable dataTable table table-striped table-bordered">

        <thead>

          <tr>

            <th>Build ID</th>
@@ -62,7 +65,11 @@

      {% if loop.last %}

        </tbody>

      </table>

-     {{ initialize_datatables() }}

+     {% if not serverside_pagination %}

+         {{ initialize_datatables() }}

+     {% else %}

+         {{ pagination_form(serverside_pagination) }}

+     {% endif %}

      {% if print_possible_states %}

        {{ build_states() }}

      {% endif %}

@@ -1,8 +1,15 @@

- {% from "_helpers.html" import copr_url, build_state, initialize_datatables %}

+ {% from "_helpers.html" import copr_url, build_state, initialize_datatables, serverside_pagination_warning %}

+ {% from "_helpers.html" import pagination_form with context %}

+ {% from "coprs/detail/_build_states.html" import build_states %}

  

- {% macro packages_table(packages) %}

+ {% macro packages_table(packages, serverside_pagination=None) %}

+ {% if not serverside_pagination %}

+ <noscript><p>WARNING!! This page is using JavaScript to filter, sort and delete builds from the table.</p></noscript>

+ {% else %}

+ {{ serverside_pagination_warning() }}

+ {% endif %}

  {% if packages %}

-   <table class="datatable table table-striped table-bordered">

+   <table class="datatable dataTable table table-striped table-bordered">

      <thead>

        <tr>

          <th>Name</th>
@@ -70,6 +77,10 @@

    <h3>No packages so far</h3>

  {% endif %}

  

+ {% if not serverside_pagination %}

  {{ initialize_datatables(order="asc") }}

- 

+ {% else %}

+ {{ pagination_form(serverside_pagination) }}

+ {% endif %}

+ {{ build_states() }}

  {% endmacro %}

@@ -1,7 +1,7 @@

  {% extends "coprs/detail.html" %}

  {% block title %}Builds for {{ copr.full_name }}{% endblock %}

  

- {% from "_helpers.html" import render_pagination %}

+ {% from "_helpers.html" import render_pagination, serverside_pagination_warning %}

  {% from "coprs/detail/_builds_table.html" import builds_table with context %}

  {% from "coprs/detail/_build_states.html" import build_states %}

  {% from "coprs/detail/_builds_forms.html" import copr_delete_builds %}
@@ -57,6 +57,11 @@

    </div>

    {% endif %}

  

+   {% if builds.items %}

+   {{ serverside_pagination_warning() }}

+   {{ builds_table(builds.items, serverside_pagination=builds) }}

+   {% else %}

    {{ builds_table(builds) }}

+   {% endif %}

  

  {% endblock %}

@@ -1,5 +1,5 @@

  {% extends "coprs/detail.html" %}

- {% from "_helpers.html" import copr_url, copr_name %}

+ {% from "_helpers.html" import copr_url, copr_name, serverside_pagination_warning  %}

  {% from "coprs/detail/_build_states.html" import build_states %}

  {% block title %}Monitor {{ copr_name(copr) }}{% endblock %}

  
@@ -26,6 +26,10 @@

  

  <h2 class="section-heading"> Build Monitor </h2>

  

+ {% if pagination.serverside_pagination %}

+ {{ serverside_pagination_warning() }}

+ {% endif %}

+ 

  <div class="panel panel-default panel-monitor-menu">

    <div class="panel-body">

      <div class="btn-group" role="group">

@@ -1,6 +1,7 @@

  {% extends "coprs/detail/monitor.html" %}

  

- {% from "_helpers.html" import package_href, build_href, build_state_text, render_monitor_table %}

+ {% from "_helpers.html" import package_href, build_href, build_state_text %}

+ {% from "_helpers.html" import render_monitor_table with context %}

  {% from "coprs/detail/_build_states.html" import build_states %}

  

  {% set selected_monitor_tab = "detailed" %}
@@ -14,5 +15,5 @@

  {% block monitor_simple_selected %}active{% endblock %}

  

  {% block monitor_table %}

- {{ render_monitor_table(copr, monitor, oses, archs, simple=False) }}

+ {{ render_monitor_table(copr, pagination, oses, archs, simple=False) }}

  {% endblock %}

@@ -1,6 +1,7 @@

  {% extends "coprs/detail/monitor.html" %}

  

- {% from "_helpers.html" import package_href, build_href, build_state_text, render_monitor_table %}

+ {% from "_helpers.html" import package_href, build_href, build_state_text %}

+ {% from "_helpers.html" import render_monitor_table with context %}

  {% from "coprs/detail/_build_states.html" import build_states %}

  

  {% set selected_monitor_tab = "simple" %}
@@ -14,5 +15,5 @@

  {% block monitor_simple_selected %}active{% endblock %}

  

  {% block monitor_table %}

- {{ render_monitor_table(copr, monitor, oses, archs, simple=True) }}

+ {{ render_monitor_table(copr, pagination, oses, archs, simple=True) }}

  {% endblock %}

@@ -22,8 +22,7 @@

  <h2 class="page-title">Project Packages</h2>

  

  {% if packages %}

-   {{ packages_table(packages) }}

-   {{ build_states() }}

+   {{ packages_table(packages, serverside_pagination=serverside_pagination) }}

  {% else %}

  <div class="blank-slate-pf">

    <div class="blank-slate-pf-icon">

@@ -1,18 +1,25 @@

  import flask

  from flask import request, render_template, stream_with_context

+ from sqlalchemy import desc

  

  from copr_common.enums import StatusEnum

  from coprs import app

  from coprs import db

  from coprs import forms

  from coprs import helpers

+ from coprs import models

  

  from coprs.logic import builds_logic

  from coprs.logic.builds_logic import BuildsLogic

  from coprs.logic.complex_logic import ComplexLogic

  from coprs.logic.coprs_logic import CoprDirsLogic

  

- from coprs.views.misc import (login_required, req_with_copr, send_build_icon)

+ from coprs.views.misc import (

+     login_required,

+     req_with_copr,

+     req_with_pagination,

+     send_build_icon,

+ )

  from coprs.views.coprs_ns import coprs_ns

  

  from coprs.exceptions import (
@@ -55,11 +62,25 @@

  @coprs_ns.route("/<username>/<coprname>/builds/")

  @coprs_ns.route("/g/<group_name>/<coprname>/builds/")

  @req_with_copr

- def copr_builds(copr):

+ @req_with_pagination

+ def copr_builds(copr, page=1):

+ 

      flashes = flask.session.pop('_flashes', [])

      dirname = flask.request.args.get('dirname')

      builds_query = builds_logic.BuildsLogic.get_copr_builds_list(copr, dirname)

-     builds = builds_query.yield_per(1000)

+ 

+     one_js_page_limit = 10000

+     if builds_query.count() > one_js_page_limit:

+         # we currently don't support filtering with server-side pagination,

+         # so order the query so the newest builds are shown first

+         builds_query = builds_query.order_by(desc(models.Build.id))

+         builds = builds_query.paginate(

+             page=page,

+             per_page=50,

+         )

+     else:

+         builds = builds_query.yield_per(1000)

+ 

      dirs = CoprDirsLogic.get_all_with_latest_submitted_build(copr.id)

  

      response = flask.Response(stream_with_context(helpers.stream_template("coprs/detail/builds.html",

@@ -35,8 +35,14 @@

  from coprs.logic.complex_logic import ComplexLogic, ReposLogic

  from coprs.logic.outdated_chroots_logic import OutdatedChrootsLogic

  

- from coprs.views.misc import (login_required, page_not_found, req_with_copr,

-                               generic_error, req_with_copr_dir)

+ from coprs.views.misc import (

+     generic_error,

+     login_required,

+     page_not_found,

+     req_with_copr,

+     req_with_copr_dir,

+     req_with_pagination,

+ )

  

  from coprs.views.coprs_ns import coprs_ns

  
@@ -1003,9 +1009,26 @@

          return flask.render_template("404.html")

  

  

- def render_monitor(copr, detailed=False):

-     monitor = builds_logic.BuildsMonitorLogic.get_monitor_data(copr)

+ @coprs_ns.route("/<username>/<coprname>/monitor/")

+ @coprs_ns.route("/<username>/<coprname>/monitor/<detailed>")

+ @coprs_ns.route("/g/<group_name>/<coprname>/monitor/")

+ @coprs_ns.route("/g/<group_name>/<coprname>/monitor/<detailed>")

+ @req_with_copr

+ @req_with_pagination

+ def copr_build_monitor(copr, detailed=False, page=1):

+     """

+     The monitor page (overview of the build status in each chroot for all the

+     packages built in given project).

+     """

+     detailed = detailed == "detailed"

+     pagination = builds_logic.BuildsMonitorLogic.get_monitor_data(

+             copr, page=page)

      oses = [chroot.os for chroot in copr.active_chroots_sorted]

+ 

+     chroot_indexer = {}

+     for index, chroot in enumerate(copr.active_chroots_sorted):

+         chroot_indexer[chroot] = index

+ 

      oses_grouped = [(len(list(group)), key) for key, group in groupby(oses)]

      archs = [chroot.arch for chroot in copr.active_chroots_sorted]

      if detailed:
@@ -1014,20 +1037,11 @@

          template = "coprs/detail/monitor/simple.html"

      return flask.Response(stream_with_context(helpers.stream_template(template,

                                   copr=copr,

-                                  monitor=monitor,

+                                  pagination=pagination,

                                   oses=oses_grouped,

                                   archs=archs,)))

  

  

- @coprs_ns.route("/<username>/<coprname>/monitor/")

- @coprs_ns.route("/<username>/<coprname>/monitor/<detailed>")

- @coprs_ns.route("/g/<group_name>/<coprname>/monitor/")

- @coprs_ns.route("/g/<group_name>/<coprname>/monitor/<detailed>")

- @req_with_copr

- def copr_build_monitor(copr, detailed=False):

-     return render_monitor(copr, detailed == "detailed")

- 

- 

  @coprs_ns.route("/<username>/<coprname>/fork/")

  @coprs_ns.route("/g/<group_name>/<coprname>/fork/")

  @login_required

@@ -13,7 +13,12 @@

      render_add_build_custom,

      render_add_build_distgit,

  )

- from coprs.views.misc import login_required, req_with_copr, send_build_icon

+ from coprs.views.misc import (

+     login_required,

+     req_with_copr,

+     req_with_pagination,

+     send_build_icon

+ )

  from coprs.logic.complex_logic import ComplexLogic

  from coprs.logic.packages_logic import PackagesLogic

  from coprs.logic.users_logic import UsersLogic
@@ -21,19 +26,35 @@

                                InsufficientRightsException, MalformedArgumentException)

  

  

+ 

  @coprs_ns.route("/<username>/<coprname>/packages/")

  @coprs_ns.route("/g/<group_name>/<coprname>/packages/")

  @req_with_copr

- def copr_packages(copr):

+ @req_with_pagination

+ def copr_packages(copr, page=1):

      flashes = flask.session.pop('_flashes', [])

+ 

+     copr_dir_id = copr.main_dir.id

+     query_packages = PackagesLogic.get_all_ordered(copr_dir_id)

+ 

+     pagination = None

+     if query_packages.count() > 1000:

+         pagination = query_packages.paginate(page=page, per_page=50)

+         packages = pagination.items

+     else:

+         packages = query_packages.all()

+ 

+     # Assign the latest builds to the package array set.

      packages = PackagesLogic.get_packages_with_latest_builds_for_dir(

-         copr.main_dir.id)

+         copr.main_dir.id, packages=packages)

+ 

      response = flask.Response(

          stream_with_context(helpers.stream_template(

              "coprs/detail/packages.html",

              copr=copr,

              packages=packages,

              flashes=flashes,

+             serverside_pagination=pagination,

          )))

      flask.session.pop('_flashes', [])

      return response

@@ -0,0 +1,26 @@

+ """

+ Several Copr WebUI pages require the server-side pagination, and the way the

+ pagination html tool works requires us to provide additional redirect routes for

+ the pages.

+ """

+ 

+ import flask

+ from coprs.views.coprs_ns import coprs_ns

+ from coprs import helpers

+ 

+ 

+ @coprs_ns.route("/<username>/<coprname>/builds/", methods=["POST"])

+ @coprs_ns.route("/g/<group_name>/<coprname>/builds/", methods=["POST"])

+ @coprs_ns.route("/<username>/<coprname>/packages/", methods=["POST"])

+ @coprs_ns.route("/g/<group_name>/<coprname>/packages/", methods=["POST"])

+ @coprs_ns.route("/<username>/<coprname>/monitor/", methods=["POST"])

+ @coprs_ns.route("/<username>/<coprname>/monitor/<detailed>", methods=["POST"])

+ @coprs_ns.route("/g/<group_name>/<coprname>/monitor/", methods=["POST"])

+ @coprs_ns.route("/g/<group_name>/<coprname>/monitor/<detailed>", methods=["POST"])

+ def copr_pagination_redirect(**_kwargs):

+     """

+     Redirect the current page to the very same page, with just the '?page=<N>'

+     argument changed.

+     """

+     to_page = flask.request.form.get('go_to_page', 1)

+     return flask.redirect(helpers.current_url(page=to_page))

@@ -21,6 +21,7 @@

  from coprs.logic.complex_logic import ComplexLogic

  from coprs.logic.users_logic import UsersLogic

  from coprs.logic.coprs_logic import CoprsLogic

+ from coprs.exceptions import ObjectNotFound

  

  

  def create_user_wrapper(username, email, timezone=None):
@@ -396,3 +397,19 @@

      if no_cache:

          response.headers['Cache-Control'] = 'public, max-age=60'

      return response

+ 

+ 

+ 

+ def req_with_pagination(f):

+     """

+     Parse 'page=' option from GET url, and place it as the argument

+     """

+     @wraps(f)

+     def wrapper(*args, **kwargs):

+         try:

+             page = flask.request.args.get('page', 1)

+             page = int(page)

+         except ValueError as err:

+             raise ObjectNotFound("Invalid pagination format") from err

+         return f(*args, page=page, **kwargs)

+     return wrapper

@@ -25,6 +25,7 @@

  

  class TestMonitor(CoprsTestCase):

  

+     @new_app_context

      def test_regression_monitor_no_copr_returned(self, f_db, f_users, f_mock_chroots):

          # https://bugzilla.redhat.com/show_bug.cgi?id=1165284

  

no initial comment

I still need to do this for packages field, and perhaps something for the monitor page, too.

Build succeeded.

Note that lists that have less than N items are not affected by this change (according to our agreement on the meeting).

2 new commits added

  • frontend: server-side pagination for too-many-builds
  • frontend: speedup /<owner>/<project>/builds/ route
3 years ago

2 new commits added

  • frontend: server-side pagination for too-many-builds
  • frontend: speedup /<owner>/<project>/builds/ route
3 years ago

Metadata Update from @praiskup:
- Pull-request tagged with: needs-work

3 years ago

Build succeeded.

4 new commits added

  • frontend: web-ui: sync package list with build list
  • frontend: paginate large package lists in web-UI
  • frontend: server-side pagination for too-many-builds
  • frontend: speedup /<owner>/<project>/builds/ route
3 years ago

4 new commits added

  • frontend: web-ui: sync package list with build list
  • frontend: web-ui: server-side pagination for too-many-builds
  • frontend: web-ui: server-side pagination for too-many-builds
  • frontend: speedup /<owner>/<project>/builds/ route
3 years ago

Build succeeded.

1 new commit added

  • backend: don't unnecessarily split the web-ui monitor route
3 years ago

Build succeeded.

Metadata Update from @praiskup:
- Pull-request untagged with: needs-work

3 years ago

Pagination of monitor page is not trivial :-( We need to construct the query using
flask-sqlalchemy, but we do a plain-text query only. I'll try to get to this later after PTO,
but it can be done in a separate pull request.

So please take a look at this.

Please take a look at this?

I don't see any issue with this but I am just curious why don't we order by desc(models.Build.id)? IMHO it should be equivalent and its index should already exist since it is a primary key ... I am surely missing something

Nitpick: The pagination here means Javascript pagination? The template for packages uses a more explicit serverside_pagination variable which I like better.

frontend: web-ui: server-side pagination for too-many-builds

There are two commits with this name. The latter one should be s/too-many-builds/too-many-packges/

Please take a look at this?

I am sorry for the late review, I was halfway through, got distracted, and the week was gone.

I made a couple of comments about minor things but overall I love this. It is amazingly fast.

msuchy: disclaimer that we are not using javascript

rebased onto 4063c2e

3 years ago

Build succeeded.

1 new commit added

  • frontend: add a warning about the server-side pagination
3 years ago

Build succeeded.

I see you fixed also the monitor page, GJ

Please take another look.

LGTM, thank you for the changes.

Thanks for taking a look!

Pull-Request has been merged by praiskup

3 years ago
Metadata
Changes Summary 22
+21
file added
frontend/coprs_frontend/alembic/versions/d5990bd4aa46_index_build_id_per_dir.py
+2 -1
file changed
frontend/coprs_frontend/coprs/__init__.py
+10 -1
file changed
frontend/coprs_frontend/coprs/context_processors.py
+15 -0
file changed
frontend/coprs_frontend/coprs/helpers.py
+71 -37
file changed
frontend/coprs_frontend/coprs/logic/builds_logic.py
+3 -2
file changed
frontend/coprs_frontend/coprs/logic/coprs_logic.py
+27 -3
file changed
frontend/coprs_frontend/coprs/logic/packages_logic.py
+10 -7
file changed
frontend/coprs_frontend/coprs/models.py
+66 -41
file changed
frontend/coprs_frontend/coprs/templates/_helpers.html
+10 -3
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html
+15 -4
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/_packages_table.html
+6 -1
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/builds.html
+5 -1
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/monitor.html
+3 -2
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/monitor/detailed.html
+3 -2
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/monitor/simple.html
+1 -2
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/packages.html
+24 -3
file changed
frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
+28 -14
file changed
frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+24 -3
file changed
frontend/coprs_frontend/coprs/views/coprs_ns/coprs_packages.py
+26
file added
frontend/coprs_frontend/coprs/views/coprs_ns/pagination_redirect.py
+17 -0
file changed
frontend/coprs_frontend/coprs/views/misc.py
+1 -0
file changed
frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py