#620 [frontend] Package.max_builds config option
Merged 5 years ago by msuchy. Opened 5 years ago by praiskup.
Unknown source max-builds-config-option  into  master

file modified
+8
@@ -541,6 +541,7 @@

              "pypi_package_version": args.packageversion,

              "spec_template": args.spec_template,

              "python_versions": args.pythonversions,

+             "max_builds": args.max_builds,

              "webhook_rebuild": ON_OFF_MAP[args.webhook_rebuild],

          }

          if args.create:
@@ -560,6 +561,7 @@

              "spec": args.spec,

              "scm_type": args.scm_type,

              "source_build_method": args.srpm_build_method,

+             "max_builds": args.max_builds,

              "webhook_rebuild": ON_OFF_MAP[args.webhook_rebuild],

          }

          if args.create:
@@ -574,6 +576,7 @@

          data = {

              "package_name": args.name,

              "gem_name": args.gem_name,

+             "max_builds": args.max_builds,

              "webhook_rebuild": ON_OFF_MAP[args.webhook_rebuild],

          }

          if args.create:
@@ -591,6 +594,7 @@

              "script_chroot": args.script_chroot,

              "script_builddeps": args.script_builddeps,

              "script_resultdir": args.script_resultdir,

+             "max_builds": args.max_builds,

              "webhook_rebuild": ON_OFF_MAP[args.webhook_rebuild],

          }

          if args.create:
@@ -999,6 +1003,10 @@

                                                     help="The copr repo for the package. Can be just name of project or even in format username/project or @groupname/project.")

      parser_add_or_edit_package_parent.add_argument("--webhook-rebuild",

                                                     choices=["on", "off"], help="Enable auto-rebuilding.")

+     parser_add_or_edit_package_parent.add_argument(

+             "--max-builds",

+             help="Keep only the specified number of the newest-by-id builds "\

+                  "(garbage collector is run daily), zero disables (default)")

  

      # PyPI edit/create

      parser_add_package_pypi = subparsers.add_parser("add-package-pypi",

@@ -2,3 +2,4 @@

  

  runuser -c '/usr/share/copr/coprs_frontend/manage.py vacuum_graphs' - copr-fe

  runuser -c '/usr/share/copr/coprs_frontend/manage.py clean_expired_project' - copr-fe

+ runuser -c '/usr/share/copr/coprs_frontend/manage.py clean_old_builds' - copr-fe

@@ -0,0 +1,18 @@

+ """

+ max package builds per copr dir

+ """

+ 

+ import sqlalchemy as sa

+ from alembic import op

+ 

+ revision = '9bc8681ed275'

+ down_revision = 'b828274ddebf'

+ 

+ 

+ def upgrade():

+     op.add_column('package', sa.Column('max_builds', sa.Integer(), nullable=True))

+     op.create_index(op.f('ix_package_max_builds'), 'package', ['max_builds'], unique=False)

+ 

+ def downgrade():

+     op.drop_index(op.f('ix_package_max_builds'), table_name='package')

+     op.drop_column('package', 'max_builds')

@@ -0,0 +1,15 @@

+ from flask_script import Command

+ from coprs import db_session_scope

+ from coprs.logic.builds_logic import BuildsLogic

+ 

+ 

+ class DeleteOldBuilds(Command):

+     """

+     This garbage collects all builds which are "obsoleted" per user

+     configuration, per models.Package.max_builds configuration.

+     """

+ 

+     # pylint: disable=method-hidden

+     def run(self):

+         with db_session_scope():

+             BuildsLogic.clean_old_builds()

@@ -500,6 +500,16 @@

              validate_chroot_blacklist,

          ],

      )

+     max_builds = wtforms.IntegerField(

+         "Max number of builds",

+         description="""Keep only the specified number of the newest-by-id builds

+         (garbage collector is run daily)""",

+         render_kw={'placeholder': 'Optional - integer, e.g. 10, zero/empty disables'},

+         validators=[

+             wtforms.validators.Optional(),

+             wtforms.validators.NumberRange(min=0, max=100)],

+         default=None,

+     )

  

  

  class PackageFormScm(BasePackageForm):

@@ -9,7 +9,7 @@

  from sqlalchemy.sql import text

  from sqlalchemy import or_

  from sqlalchemy import and_

- from sqlalchemy import func

+ from sqlalchemy import func, desc

  from sqlalchemy.sql import false,true

  from werkzeug.utils import secure_filename

  from sqlalchemy import bindparam, Integer, String
@@ -1107,6 +1107,39 @@

      def filter_by_package_name(cls, query, package_name):

          return query.join(models.Package).filter(models.Package.name == package_name)

  

+     @classmethod

+     def clean_old_builds(cls):

+         dirs = (

+             db.session.query(

+                 models.CoprDir.id,

+                 models.Package.id,

+                 models.Package.max_builds)

+             .join(models.Build)

+             .join(models.Package)

+             .filter(models.Package.max_builds > 0)

You are querying using this column. Can you create index on that column as well?

+             .group_by(

+                 models.CoprDir.id,

+                 models.Package.max_builds,

+                 models.Package.id)

+             .having(func.count(models.Build.id) > models.Package.max_builds)

+         )

+ 

+         for dir_id, package_id, limit in dirs.all():

+             delete_builds = (

+                 models.Build.query.filter(

+                     models.Build.copr_dir_id==dir_id,

+                     models.Build.package_id==package_id)

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

+                 .offset(limit)

+                 .all()

+             )

+ 

+             for build in delete_builds:

+                 try:

+                     cls.delete_build(build.copr.user, build)

+                 except ActionInProgressException:

+                     # postpone this one to next day run

+                     log.error("Build(id={}) delete failed, unfinished action.".format(build.id))

  

  class BuildChrootsLogic(object):

      @classmethod

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

  

  from sqlalchemy import outerjoin

  from sqlalchemy.ext.associationproxy import association_proxy

- from sqlalchemy.orm import column_property

+ from sqlalchemy.orm import column_property, validates

  from six.moves.urllib.parse import urljoin

  from libravatar import libravatar_url

  import zlib
@@ -590,6 +590,13 @@

      # enable networking during a build process

      enable_net = db.Column(db.Boolean, default=False, server_default="0", nullable=False)

  

+     # don't keep more builds of this package per copr-dir

+     max_builds = db.Column(db.Integer, index=True)

+ 

+     @validates('max_builds')

+     def validate_max_builds(self, field, value):

+         return None if value == 0 else value

+ 

      builds = db.relationship("Build", order_by="Build.id")

  

      # relations

@@ -14,6 +14,10 @@

              <small class="text-muted pficon pficon-info"></small> {{line|safe}}

            </li>

            {% endfor %}

+         {% elif field.description %}

+           <li class="help-block">

+             <small class="text-muted pficon pficon-info"></small> {{field.description|safe}}

+           </li>

          {% endif %}

          {% if field.errors %}

            {% for error in field.errors %}

@@ -62,6 +62,7 @@

                  placeholder="Optional - comma-separated list of wildcard-patterns, e.g.  fedora-*, *-i386",

                  info="What chroots should be skipped for this package, by default we build for all.",

  )}}

+ {{ render_field(form.max_builds)}}

  {% endmacro %}

  

  {% macro render_anitya_autorebuild(form) %}

@@ -898,6 +898,8 @@

          package.source_json = form.source_json

          if "webhook_rebuild" in formdata:

              package.webhook_rebuild = form.webhook_rebuild.data

+         if "max_builds" in formdata:

+             package.max_builds = form.max_builds.data

  

          db.session.add(package)

          db.session.commit()

@@ -158,6 +158,7 @@

      data = package.source_json_dict

      data["webhook_rebuild"] = package.webhook_rebuild

      data["chroot_blacklist"] = package.chroot_blacklist_raw

+     data["max_builds"] = package.max_builds

  

      if package.has_source_type_set and not source_type_text:

          source_type_text = package.source_type_text
@@ -229,6 +230,7 @@

              package.webhook_rebuild = form.webhook_rebuild.data

              package.source_json = form.source_json

              package.chroot_blacklist_raw = form.chroot_blacklist.data

+             package.max_builds = form.max_builds.data

  

              db.session.add(package)

              db.session.commit()

@@ -41,6 +41,7 @@

      "notify_outdated_chroots": "NotifyOutdatedChrootsCommand",

      "delete_outdated_chroots": "DeleteOutdatedChrootsCommand",

      "clean_expired_projects": "CleanExpiredProjectsCommand",

+     "clean_old_builds": "DeleteOldBuilds",

  }

  

  if os.getuid() == 0:

@@ -233,3 +233,26 @@

          assert self.b1.status == StatusEnum("succeeded")

          assert self.b3.status == StatusEnum("failed")

          assert type(BuildsLogic.mark_as_failed(self.b3.id)) == models.Build

+ 

+     def test_build_garbage_collector_works(self, f_users, f_coprs,

+             f_mock_chroots, f_builds, f_db):

+ 

+         assert len(self.db.session.query(models.Build).all()) == 4

+ 

+         p = models.Package.query.filter_by(name='whatsupthere-world').first()

+         p.max_builds = 1

+         self.db.session.add(p)

+ 

+         assert len(models.Package.query.all()) == 3

+         BuildsLogic.clean_old_builds()

+ 

+         # we can not delete not-yet finished builds!

+         assert len(self.db.session.query(models.Build).all()) == 4

+ 

+         for bch in self.b3.build_chroots:

+             bch.status = StatusEnum('succeeded')

+             self.db.session.add(bch)

+ 

+         assert len(self.db.session.query(models.Build).all()) == 4

+         BuildsLogic.clean_old_builds()

+         assert len(self.db.session.query(models.Build).all()) == 3

    [frontend] Package.max_builds config option

    Allow users to configure how many builds are to be kept in
    database (and thus in turn also in repositories) per package.

    Note that this has nothing to do with prunerepo cron
    job & friends.  If user configures Package.max_builds, the builds
    are removed no matter what.

    The consequence of this change is that if user isn't able to
    handle ENVRs properly, the RPM repository might end up with no
    built binary RPMs for the Package.  Example situation:
    (a) prunerepo removes all the old RPMs per ENVRs even though those
    don't really come from the newest package builds and
    (b) the build garbage collector removes the older package builds
    per 'max_builds' config option.

    Since people using the new feature need to know what they are
    doing (and thus they are probably OK to keep NVR rising) this
    isn't a real blocker.

rebased onto 30067dc02c25b39ee93af73cd860b3ff8c5a5630

5 years ago

At this point WIP, tests need to be added and optionally a cli option as well.

rebased onto ab5068a768226f42613509b04e113e232a47b5f1

5 years ago

Can I continue on this?

Metadata Update from @praiskup:
- Pull-request tagged with: question

5 years ago

rebased onto 8154129616a940653c94444d8248f398d05a9cb0

5 years ago

I forgot to add the cron script, rebased.

Note from Mirek, add hint that we delete builds daily.

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

5 years ago

rebased onto bde06faf1750680a0339aa14079f0fc5b6e71ce7

5 years ago

rebased onto 29fead469b06070395543bda126b548977503123

5 years ago

Screenshot:
https://praiskup.fedorapeople.org/pull-requets/copr/620/edit-package.png

I keep the needs-work since I am not able to fix API for this today; feel free to merge, or +1 if you think it is not necessary.

rebased onto a5ebcf7f35b782001052f4e90a0c9bb5cd6bf2fe

5 years ago

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

5 years ago

rebased onto 59eda1310d5a8263cc9302c3e392d58803be3f58

5 years ago

Slightly updated output:
https://praiskup.fedorapeople.org/pull-requets/copr/620/edit-package-2.png

Cli help output:

 --max-builds MAX_BUILDS
                        Keep only the specified number of the newest-by-id
                        builds (garbage collector is run daily), zero disables
                        (default)

You are querying using this column. Can you create index on that column as well?

rebased onto fa112845bacfb4283224a5b21e469f93f6c86289

5 years ago

Can you create index on that column as well?

Done, thanks.

+1
but failed to rebase

rebased onto 45220f6

5 years ago

Pull-Request has been merged by msuchy

5 years ago