copr / copr

Clone

#1237 frontend: module enable option for chroot settings
Merged 4 years ago by praiskup. Opened 4 years ago by thrnciar.
copr/ thrnciar/copr input-modul-enable  into  master

file added
+20 
@@ -0,0 +1,20 @@ 
 

+ """
 

+ add module_toggle to copr_chroot
 

+ 

+ Revision ID: 67ba91dda3e3
 

+ Revises: 2561c13a3556
 

+ Create Date: 2020-01-17 10:48:22.092706
 

+ """
 

+ 

+ import sqlalchemy as sa
 

+ from alembic import op
 

+ 

+ 

+ revision = '67ba91dda3e3'
 

+ down_revision = '2561c13a3556'
 

+ 

+ def upgrade():
 

+     op.add_column('copr_chroot', sa.Column('module_toggle', sa.Text(), nullable=True))
 

+ 

+ def downgrade():
 

+     op.drop_column('copr_chroot', 'module_toggle')

file modified
+43 
@@ -22,6 +22,7 @@ 
 

  from coprs.logic.dist_git_logic import DistGitLogic
 

  from coprs import exceptions
 

  

+ from wtforms import ValidationError
 

  

  FALSE_VALUES = {False, "false", ""}
 

  
@@ -165,6 +166,32 @@ 
 

                          message=self.message)
 

          validator(form, field)
 

  

+ class ModuleEnableNameValidator(object):
 

+ 

+     def __call__(self, form, field):
 

+         for module in form.module_toggle.data.split(","):
 

+             if module == "":
 

+                 return True
 

+ 

+             try:
 

+                 module_name, stream = module.strip().split(":")
 

+             except ValueError:
 

+                 raise ValidationError(
 

+                     message="Module name '{0}' must consist of two parts separated with colon.\
 

+                          Eg. module:stream"
 

+                 .format(module))
 

+ 

+             pattern = re.compile(re.compile(r"^([a-zA-Z0-9-_!][^\ ]*)$"))
 

+             if pattern.match(module_name) == None:
 

+                 raise ValidationError(
 

+                     message="Module name '{0}' must contain only letters, digits, dashes, underscores."
 

+                 .format(module_name))
 

+ 

+             if pattern.match(stream) == None:
 

+                 raise ValidationError(
 

+                     message="Stream part of module name '{0}' must contain only letters,\
 

+                         digits, dashes, underscores."
 

+                 .format(stream))
 

  

  class ChrootsValidator(object):
 

      def __call__(self, form, field):
 
@@ -220,6 +247,17 @@ 
 

          regex = re.compile(r"\s+")
 

          return regex.sub(lambda x: '\n', result)
 

  

+ class StringWhiteCharactersFilter(object):
 

+ 

+     def __call__(self, value):
 

+         if not value:
 

+             return ''
 

+ 

+         modules = [module.strip() for module in value.split(",")]
 

+         # to remove empty strings
 

+         modules = [m for m in modules if m]
 

+ 

+         return ", ".join(module for module in modules if module != "")
 

  

  class ValueToPermissionNumberFilter(object):
 

  
@@ -1006,6 +1044,11 @@ 
 

  

      comps = FileField("comps_xml")
 

  

+     module_toggle = wtforms.StringField("Enable module",
 

+                                         validators=[ModuleEnableNameValidator()],
 

+                                         filters=[StringWhiteCharactersFilter()]
 

+                                         )
 

+ 

      with_opts = wtforms.StringField("With options")
 

      without_opts = wtforms.StringField("Without options")

file modified
+8 -5 
@@ -632,7 +632,7 @@ 
 

      @classmethod
 

      def create_chroot(cls, user, copr, mock_chroot, buildroot_pkgs=None, repos=None, comps=None, comps_name=None,
 

                        with_opts="", without_opts="",
 

-                       delete_after=None, delete_notify=None):
 

+                       delete_after=None, delete_notify=None, module_toggle=""):
 

          """
 

          :type user: models.User
 

          :type mock_chroot: models.MockChroot
 
@@ -647,12 +647,12 @@ 
 

  

          chroot = models.CoprChroot(copr=copr, mock_chroot=mock_chroot)
 

          cls._update_chroot(buildroot_pkgs, repos, comps, comps_name, chroot,
 

-                            with_opts, without_opts, delete_after, delete_notify)
 

+                            with_opts, without_opts, delete_after, delete_notify, module_toggle)
 

          return chroot
 

  

      @classmethod
 

      def update_chroot(cls, user, copr_chroot, buildroot_pkgs=None, repos=None, comps=None, comps_name=None,
 

-                       with_opts="", without_opts="", delete_after=None, delete_notify=None):
 

+                       with_opts="", without_opts="", delete_after=None, delete_notify=None, module_toggle=""):
 

          """
 

          :type user: models.User
 

          :type copr_chroot: models.CoprChroot
 
@@ -662,12 +662,12 @@ 
 

              "Only owners and admins may update their projects.")
 

  

          cls._update_chroot(buildroot_pkgs, repos, comps, comps_name,
 

-                            copr_chroot, with_opts, without_opts, delete_after, delete_notify)
 

+                            copr_chroot, with_opts, without_opts, delete_after, delete_notify, module_toggle)
 

          return copr_chroot
 

  

      @classmethod
 

      def _update_chroot(cls, buildroot_pkgs, repos, comps, comps_name,
 

-                        copr_chroot, with_opts, without_opts, delete_after, delete_notify):
 

+                        copr_chroot, with_opts, without_opts, delete_after, delete_notify, module_toggle):
 

          if buildroot_pkgs is not None:
 

              copr_chroot.buildroot_pkgs = buildroot_pkgs
 

  
@@ -691,6 +691,9 @@ 
 

          if delete_notify is not None:
 

              copr_chroot.delete_notify = delete_notify
 

  

+         if module_toggle is not None:
 

+             copr_chroot.module_toggle = module_toggle
 

+ 

          db.session.add(copr_chroot)
 

  

      @classmethod

file modified
+12 
@@ -1282,6 +1282,8 @@ 
 

      comps_zlib = db.Column(db.LargeBinary(), nullable=True)
 

      comps_name = db.Column(db.String(127), nullable=True)
 

  

+     module_toggle = db.Column(db.Text, nullable=True)
 

+ 

      with_opts = db.Column(db.Text, default="", server_default="", nullable=False)
 

      without_opts = db.Column(db.Text, default="", server_default="", nullable=False)
 

  
@@ -1333,6 +1335,16 @@ 
 

          days = (self.delete_after - now).days
 

          return days if days > 0 else 0
 

  

+     @property
 

+     def module_toggle_array(self):
 

+         if not self.module_toggle:
 

+             return []
 

+         module_enable = []
 

+         for m in self.module_toggle.split(','):
 

+             if m[0] != "!":
 

+                 module_enable.append(m)

good for now, thanks. In future we'll fix this to return list of pairs ('enable/disable', 'module')

 
+         return module_enable
 

+ 

      def to_dict(self):
 

          options = {"__columns_only__": [
 

              "buildroot_pkgs", "repos", "comps_name", "copr_id", "with_opts", "without_opts"

file modified
+8 
@@ -33,6 +33,14 @@ 
 

      {{ render_field(form.repos, rows=5, cols=50, placeholder='Optional - URL to additional yum repos, which can be used during build. Space separated. This should be baseurl from .repo file. E.g.: http://copr-be.cloud.fedoraproject.org/results/rhughes/f20-gnome-3-12/fedora-$releasever-$basearch/') }}
 

  

      {{ render_field(
 

+         form.module_toggle,
 

+         size=80,
 

+         info='You can specify modules which will be enabled for builds in the given chroot.',
 

+         placeholder='module:stream, module1:stream1'
 

+        )
 

+     }}
 

+ 

+     {{ render_field(
 

          form.with_opts,
 

          size=80,
 

          info='You can specify rpmbuild --with options here for builds in the given chroot.',

file modified
+8 -1 
@@ -9,7 +9,7 @@ 
 

  from coprs.logic.builds_logic import BuildsLogic
 

  from coprs.logic.complex_logic import ComplexLogic, BuildConfigLogic
 

  from coprs.logic.packages_logic import PackagesLogic
 

- from coprs.logic.coprs_logic import MockChrootsLogic
 

+ from coprs.logic.coprs_logic import MockChrootsLogic, CoprChrootsLogic
 

  from coprs.exceptions import MalformedArgumentException, ObjectNotFound
 

  

  from coprs.views import misc
 
@@ -92,6 +92,12 @@ 
 

  

      build_record = None
 

      try:
 

+         copr_chroot = CoprChrootsLogic.get_by_name_safe(task.build.copr, task.mock_chroot.name)
 

+         enabled_disabled_modules = []
 

+         for module in copr_chroot.module_toggle_array:
 

+             if module:
 

+                 enabled_disabled_modules.append({"enable": module})
 

+ 

          build_record = {
 

              "task_id": task.task_id,
 

              "build_id": task.build.id,
 
@@ -114,6 +120,7 @@ 
 

              "package_name": task.build.package.name,
 

              "package_version": task.build.pkg_version,
 

              "uses_devel_repo": task.build.copr.devel_mode,
 

+             "modules": {'toggle': enabled_disabled_modules},
 

          }
 

          if short:
 

              return build_record

file modified
+4 -2 
@@ -29,7 +29,8 @@ 
 

      # form = forms.ChrootForm(buildroot_pkgs=copr.buildroot_pkgs(chroot))
 

  

      form = forms.ChrootForm(buildroot_pkgs=chroot.buildroot_pkgs, repos=chroot.repos,
 

-                             with_opts=chroot.with_opts, without_opts=chroot.without_opts)
 

+                             module_toggle=chroot.module_toggle, with_opts=chroot.with_opts,
 

+                             without_opts=chroot.without_opts)
 

      # FIXME - test if chroot belongs to copr
 

      if flask.g.user.can_build_in(copr):
 

          return render_template("coprs/detail/edit_chroot.html",
 
@@ -73,7 +74,8 @@ 
 

                      form.buildroot_pkgs.data,
 

                      form.repos.data,
 

                      comps=comps_xml, comps_name=comps_name,
 

-                     with_opts=form.with_opts.data, without_opts=form.without_opts.data
 

+                     with_opts=form.with_opts.data, without_opts=form.without_opts.data,
 

+                     module_toggle=form.module_toggle.data
 

                  )
 

  

              elif action == "delete_comps":

file modified
+24 -1 
@@ -8,7 +8,7 @@ 
 

  

  from copr_common.enums import ActionTypeEnum
 

  from coprs import app
 

- from coprs.forms import PinnedCoprsForm
 

+ from coprs.forms import PinnedCoprsForm, ChrootForm, ModuleEnableNameValidator
 

  from coprs.logic.actions_logic import ActionsLogic
 

  from coprs.logic.coprs_logic import CoprsLogic, CoprChrootsLogic, PinnedCoprsLogic
 

  from coprs.logic.users_logic import UsersLogic
 
@@ -211,3 +211,26 @@ 
 

          ComplexLogic.delete_copr(self.c2, admin_action=True)
 

          assert set(CoprsLogic.get_multiple_by_username(self.u2.name)) == {self.c3}
 

          assert set(PinnedCoprsLogic.get_by_owner(self.u2)) == {pc2}
 

+ 

+ class TestChrootFormLogic(CoprsTestCase):
 

+ 

+     def test_module_toggle_format(self):
 

+         with app.app_context():
 

+             form = ChrootForm()
 

+             form.module_toggle.data = "module:stream"
 

+             assert form.validate()
 

+ 

+             form.module_toggle.data = ""
 

+             assert form.validate()
 

+ 

+             form.module_toggle.data = "module:stream, module1:stream1"
 

+             assert form.validate()
 

+ 

+             form.module_toggle.data = "module"
 

+             assert False == form.validate()
 

+ 

+             form.module_toggle.data = "module 1:stream"
 

+             assert False == form.validate()
 

+ 

+             form.module_toggle.data = "module: stream"
 

+             assert False == form.validate()

file modified
+29 -1 
@@ -1,12 +1,40 @@ 
 

  import json
 

  

- from unittest import mock
 

+ from unittest import mock, skip
 

  

  from copr_common.enums import BackendResultEnum, StatusEnum
 

  from tests.coprs_test_case import CoprsTestCase, new_app_context
 

  from coprs.logic.builds_logic import BuildsLogic
 

  

  

+ class TestGetBuildTask(CoprsTestCase):
 

+ 

+     def test_module_name_empty(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):
 

+         self.c1.copr_chroots[0].module_toggle = ""
 

+         r = self.tc.get("/backend/get-build-task/" + str(self.b2.id) + "-fedora-18-x86_64", headers=self.auth_header).data
 

+         data = json.loads(r.decode("utf-8"))
 

+         assert data['modules']['toggle'] == []
 

+ 

+     def test_module_name_enable(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):
 

+         self.c1.copr_chroots[0].module_toggle = "XXX"
 

+         r = self.tc.get("/backend/get-build-task/" + str(self.b2.id) + "-fedora-18-x86_64", headers=self.auth_header).data
 

+         data = json.loads(r.decode("utf-8"))
 

+         assert data['modules']['toggle'] == [{'enable': 'XXX'}]
 

+ 

+     @skip("Modules disable not implemented yet.")
 

+     def test_module_name_disable(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):
 

+         self.c1.copr_chroots[0].module_toggle = "!XXX"
 

+         r = self.tc.get("/backend/get-build-task/" + str(self.b2.id) + "-fedora-18-x86_64", headers=self.auth_header).data
 

+         data = json.loads(r.decode("utf-8"))
 

+         assert data['modules']['toggle'] == [{'disable': '!XXX'}]
 

+ 

+     @skip("Modules disable not implemented yet.")
 

+     def test_module_name_many_modules(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):
 

+         self.c1.copr_chroots[0].module_toggle = "!XXX,YYY,ZZZ"
 

+         r = self.tc.get("/backend/get-build-task/" + str(self.b2.id) + "-fedora-18-x86_64", headers=self.auth_header).data
 

+         data = json.loads(r.decode("utf-8"))
 

+         assert data['modules']['toggle'] == [{'disable': '!XXX'}, {'enable': 'YYY'}, {'enable': 'ZZZ'}]
 

+ 

  class TestWaitingBuilds(CoprsTestCase):
 

  

      def test_no_pending_builds(self):

Metadata Update from @thrnciar:
- Pull-request tagged with: wip
4 years ago

Metadata Update from @praiskup:
- Pull-request untagged with: wip
- Pull-request tagged with: needs-tests
4 years ago

rebased onto 6695606f4c87b8fa6a7e22288f6ce07e641de6ec
4 years ago

[copr-build]

rebased onto d6f7959bc87a63fc322942e046e2ed6e88d2e5a8
4 years ago

rebased onto e3d14f0312e01183c09bda476f477684d52aa822
4 years ago

Metadata Update from @thrnciar:
- Pull-request untagged with: needs-tests
4 years ago

here could be nullable=True

I'd do ", " join so it is better for humans to read

 
@property
def module_enable_array(self): 
    if not self.module_enable:
        return []
    return [m.strip() for m in self.module_enable.split(',')]

and perhaps module_toggle, since it will in future support also disabling

rebased onto 97b43c32698fd0ec98280aaeeb5bf624c58ce699
4 years ago

Metadata Update from @praiskup:
- Pull-request tagged with: release-blocker
4 years ago

rebased onto 4ae28afffb3326e54b18ddb968ccd4406d81ce2b
4 years ago

rebased onto 94e12e12ea93bb92080d2a1be280fbd99d86f502
4 years ago

rebased onto 922d5c8ed9540473a8c3bc31a02bf580b004057f
4 years ago

this is dupe

either e.g. or just put there "module:stream, module1:stream1"

You want to filter out the exclamation mark from the module name at this point.

I'd prefer something like: Module name '{}' is invalid, it must contain ... so users know what module they did typo in.

If you catch the ValueError, I think it is fine to just do .split(':'), because if there were multiple colons you would catch it as well

Similar here, all the ValidationErrors should say what string we are grumble about.

I don't think you test anything here, unless you assert form.validate().

rebased onto 10b5d8c952b0ad537d5aa5f2ec7663cfa16190c0
4 years ago

rebased onto a6a4ab7c21ea8b899f393fa3ff150580d40fb6eb
4 years ago

rebased onto e33047666c2bfd79aa397ed284bd9d97ae4f3a78
4 years ago

good for now, thanks. In future we'll fix this to return list of pairs ('enable/disable', 'module')

please add modules = [m for m in modules if m]

rebased onto f7384fb
4 years ago

Thanks! Some stylistic patch will follow, but LGTM. Merging early since we need this before release.

Commit ddb706f fixes this pull-request

Pull-Request has been merged by praiskup
4 years ago
Changes Summary 9
+20
file added
frontend/coprs_frontend/alembic/schema/versions/67ba91dda3e3_add_module_toggle_to_copr_chroot.py
+43 -0
file changed
frontend/coprs_frontend/coprs/forms.py
+8 -5
file changed
frontend/coprs_frontend/coprs/logic/coprs_logic.py
+12 -0
file changed
frontend/coprs_frontend/coprs/models.py
+8 -0
file changed
frontend/coprs_frontend/coprs/templates/coprs/detail/edit_chroot.html
+8 -1
file changed
frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py
+4 -2
file changed
frontend/coprs_frontend/coprs/views/coprs_ns/coprs_chroots.py
+24 -1
file changed
frontend/coprs_frontend/tests/test_logic/test_coprs_logic.py
+29 -1
file changed
frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.