copr-devel March 2013

copr-devel@lists.fedorahosted.org

10 participants
39 discussions

[copr] master: Start moving authorization from views to logic (8fda311)
by bkabrda＠fedoraproject.org 28 Mar '13

28 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit 8fda311b272d73e41ecccb537ceee039ecf2ec67 Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Thu Mar 28 12:09:38 2013 +0100 Start moving authorization from views to logic - move copr updating logic - improve the ActionInProgress exception so that we can print it as any other exception, while still making the Action accessible if needed >--------------------------------------------------------------- coprs_frontend/coprs/exceptions.py | 9 +++++++++ coprs_frontend/coprs/logic/builds_logic.py | 3 ++- coprs_frontend/coprs/logic/coprs_logic.py | 20 ++++++++++++++++---- .../coprs/views/coprs_ns/coprs_builds.py | 2 +- .../coprs/views/coprs_ns/coprs_general.py | 10 +++------- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/coprs_frontend/coprs/exceptions.py b/coprs_frontend/coprs/exceptions.py index da744b2..f1954b2 100644 --- a/coprs_frontend/coprs/exceptions.py +++ b/coprs_frontend/coprs/exceptions.py @@ -17,3 +17,12 @@ class ActionInProgressException(BaseException): def __init__(self, msg, action): self.msg = msg self.action = action + + def __unicode__(self): + return self.formatted_msg() + + def __str__(self): + return self.__unicode__() + + def formatted_msg(self): + return self.msg.format(action=self.action) diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py index 247c01a..2a91f95 100644 --- a/coprs_frontend/coprs/logic/builds_logic.py +++ b/coprs_frontend/coprs/logic/builds_logic.py @@ -57,7 +57,8 @@ class BuildsLogic(object): @classmethod def add(cls, user, pkgs, copr): - coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr) + coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr, + 'Can\'t build while there is an operation in progress: {action}') build = models.Build( pkgs=pkgs, copr=copr, diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py index 1a83147..2b4b2d9 100644 --- a/coprs_frontend/coprs/logic/coprs_logic.py +++ b/coprs_frontend/coprs/logic/coprs_logic.py @@ -92,7 +92,9 @@ class CoprsLogic(object): @classmethod def update(cls, user, copr, check_for_duplicates = True): - cls.raise_if_unfinished_action(user, copr) + cls.raise_if_unfinished_action(user, copr, + 'Can\'t change this Copr name, another operation is in progress: {action}') + cls.raise_if_cant_update(user, copr, 'Only owners and admins may update their Coprs.') existing = cls.exists_for_user(copr.owner, copr.name).first() if existing: @@ -115,7 +117,8 @@ class CoprsLogic(object): def delete(cls, user, copr, check_for_duplicates=True): # for the time being, we authorize user to do this in view... # TODO: do we want to dump the information somewhere, so that we can search it in future? - cls.raise_if_unfinished_action(user, copr) + cls.raise_if_unfinished_action(user, copr, + 'Can\'t delete this Copr, another operation is in progress: {action}') action = models.Action(action_type=helpers.ActionTypeEnum('delete'), object_type='copr', object_id=copr.id, @@ -149,13 +152,22 @@ class CoprsLogic(object): return actions @classmethod - def raise_if_unfinished_action(cls, user, copr): + def raise_if_unfinished_action(cls, user, copr, message): """This method raises ActionInProgressException if given copr has an unfinished action. Returns None otherwise. """ unfinished_actions = cls.unfinished_actions_for(user, copr).all() if unfinished_actions: - raise exceptions.ActionInProgressException('Action in progress on this copr.', unfinished_actions[0]) + raise exceptions.ActionInProgressException(message, unfinished_actions[0]) + + @classmethod + def raise_if_cant_update(cls, user, copr, message): + """This method raises InsufficientRightsException if given user cant update + given copr. Returns None otherwise. + """ + # TODO: this is a bit inconsistent - shouldn't the user method be called can_update? + if not user.can_edit(copr): + raise exceptions.InsufficientRightsException(message) class CoprPermissionsLogic(object): diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py index 1f91e49..a6871cd 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -59,7 +59,7 @@ def copr_new_build(username, coprname): build.memory_reqs = form.memory_reqs.data build.timeout = form.timeout.data except exceptions.ActionInProgressException as e: - flask.flash('Can\'t build in this Copr, there is an operation in progress: {0}'.format(e.action)) + flask.flash(e) db.session.rollback() else: flask.flash("Build was added") diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index 6da142e..adba1d0 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -164,10 +164,6 @@ def copr_edit(username, coprname, form=None): def copr_update(username, coprname): form = forms.CoprFormFactory.create_form_cls()() copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first() - # only owner can update a copr - if not flask.g.user.can_edit(copr): - flask.flash('Only owners and admins may update their Coprs.') - return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = form.name.data)) if form.validate_on_submit(): # we don't change owner (yet) @@ -179,8 +175,8 @@ def copr_update(username, coprname): try: coprs_logic.CoprsLogic.update(flask.g.user, copr, check_for_duplicates = False) # form validation checks for duplicates - except exceptions.ActionInProgressException as e: - flask.flash('Can\'t change this Copr name, there is another operation in progress: {0}'.format(e.action)) + except (exceptions.ActionInProgressException, exceptions.InsufficientRightsException) as e: + flask.flash(e) db.session.rollback() else: flask.flash('Copr was updated successfully.') @@ -252,7 +248,7 @@ def copr_delete(username, coprname): coprs_logic.CoprsLogic.delete(flask.g.user, copr) except exceptions.ActionInProgressException as e: db.session.rollback() - flask.flash('Can\'t manipulate this Copr, there is another operation in progress: {0}'.format(e.action)) + flask.flash(e) return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname)) else: db.session.commit()

1 0

[copr] master: Allow deleting coprs (010246d)
by bkabrda＠fedoraproject.org 27 Mar '13

27 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit 010246d4e7351c9113cf1c3b4cee7aa20ae26af5 Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Wed Mar 27 14:46:33 2013 +0100 Allow deleting coprs >--------------------------------------------------------------- coprs_frontend/coprs/forms.py | 3 + coprs_frontend/coprs/logic/coprs_logic.py | 17 ++++++++ .../coprs/templates/coprs/_coprs_forms.html | 17 ++++++++ coprs_frontend/coprs/templates/coprs/detail.html | 5 ++ .../coprs/templates/coprs/detail/delete.html | 14 +++++++ .../coprs/views/coprs_ns/coprs_general.py | 24 +++++++++++ coprs_frontend/coprs/whoosheers.py | 4 ++ .../tests/test_views/test_coprs_ns/test_general.py | 41 ++++++++++++++++++++ 8 files changed, 125 insertions(+), 0 deletions(-) diff --git a/coprs_frontend/coprs/forms.py b/coprs_frontend/coprs/forms.py index 5c80c33..7c7cf35 100644 --- a/coprs_frontend/coprs/forms.py +++ b/coprs_frontend/coprs/forms.py @@ -123,6 +123,9 @@ class CoprFormFactory(object): return F +class CoprDeleteForm(wtf.Form): + verify = wtf.TextField('Confirm deleting by typing "yes"', + validators=[wtf.Required(), wtf.Regexp(r'^yes$', message='Type "yes" - without the quotes, lowercase.')]) class BuildForm(wtf.Form): pkgs = wtf.TextAreaField('Pkgs', diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py index 29aa6b0..1a83147 100644 --- a/coprs_frontend/coprs/logic/coprs_logic.py +++ b/coprs_frontend/coprs/logic/coprs_logic.py @@ -112,6 +112,23 @@ class CoprsLogic(object): db.session.add(copr) @classmethod + def delete(cls, user, copr, check_for_duplicates=True): + # for the time being, we authorize user to do this in view... + # TODO: do we want to dump the information somewhere, so that we can search it in future? + cls.raise_if_unfinished_action(user, copr) + action = models.Action(action_type=helpers.ActionTypeEnum('delete'), + object_type='copr', + object_id=copr.id, + old_value='{0}/{1}'.format(copr.owner.name, copr.name), + new_value='', + created_on=int(time.time())) + + db.session.add(action) + db.session.delete(copr) + + return copr + + @classmethod def exists_for_user(cls, user, coprname): existing = models.Copr.query.filter(models.Copr.name==coprname).\ filter(models.Copr.owner_id==user.id) diff --git a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html index b3f6742..a0f55e3 100644 --- a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html +++ b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html @@ -31,6 +31,23 @@ </form> {% endmacro %} +{% macro copr_delete_form(form, copr) %} + <form action="{{ url_for('coprs_ns.copr_delete', username=copr.owner.name, coprname=copr.name) }}" method="post"> + <dl> + {{ form.csrf_token }} + <dd> + {% if form.verify.errors %} + {% for error in form.verify.errors %} + <p class="form-error">{{ error }}</p> + {% endfor %} + {% endif %} + </dd> + {{ form.verify }} + <dt><input type="submit" value="Delete"></dt> + </dl> + </form> +{% endmacro %} + {% macro copr_permissions_form(form, copr, permissions) %} {% if permissions %} <form action="{{ url_for('coprs_ns.copr_update_permissions', username=copr.owner.name, coprname=copr.name) }}" method=post> diff --git a/coprs_frontend/coprs/templates/coprs/detail.html b/coprs_frontend/coprs/templates/coprs/detail.html index ef0f097..749ffb6 100644 --- a/coprs_frontend/coprs/templates/coprs/detail.html +++ b/coprs_frontend/coprs/templates/coprs/detail.html @@ -27,6 +27,11 @@ <a href="{{ url_for('coprs_ns.copr_edit', username = copr.owner.name, coprname = copr.name) }}">Edit</a> </li> {% endif %} + {% if g.user and g.user == copr.owner %} + <li class="{% block delete_selected %}unselected{% endblock %}"> + <a href="{{ url_for('coprs_ns.copr_delete', username=copr.owner.name, coprname=copr.name) }}">Delete</a> + </li> + {% endif %} </ul> </div> {% block detail_body %}{% endblock %} diff --git a/coprs_frontend/coprs/templates/coprs/detail/delete.html b/coprs_frontend/coprs/templates/coprs/detail/delete.html new file mode 100644 index 0000000..0e65de0 --- /dev/null +++ b/coprs_frontend/coprs/templates/coprs/detail/delete.html @@ -0,0 +1,14 @@ +{% extends "coprs/detail.html" %} +{% block title %}Delete {{ copr.owner.name }}/{{ copr.name }}?{% endblock %} +{% block delete_selected %}selected{% endblock %} +{% from "coprs/_coprs_forms.html" import copr_delete_form %} + +{% block detail_body %} + <p>If you really want to delete this Copr, you'll have to answer this riddle:</p> + <p>{{ range(5)|random }}.{{ range(10)|random }} hens lay {{ range(5)|random }}.{{ range(10)|random }} eggs in + {{ range(5)|random }}.{{ range(10)|random }} days. How many eggs do {{ range(5)|random }}.{{ range(10)|random }} + hens lay in {{ range(5)|random }}.{{ range(10)|random }} days?</p> + <p>Ok, kidding, just type "<strong>yes</strong>" into the below box.</p> + {{ copr_delete_form(form, copr) }} + +{% endblock %} diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index fc1a0ad..6da142e 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -236,3 +236,27 @@ def copr_update_permissions(username, coprname): flask.flash('Copr permissions were updated successfully.') return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name)) + +(a)coprs_ns.route('/detail/<username>/<coprname>/delete/', methods=['GET', 'POST']) +@login_required +def copr_delete(username, coprname): + form = forms.CoprDeleteForm() + copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first() + # only owner can delete a copr + if flask.g.user != copr.owner: + flask.flash('Only owners may delete their Coprs.') + return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname)) + + if form.validate_on_submit(): + try: + coprs_logic.CoprsLogic.delete(flask.g.user, copr) + except exceptions.ActionInProgressException as e: + db.session.rollback() + flask.flash('Can\'t manipulate this Copr, there is another operation in progress: {0}'.format(e.action)) + return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname)) + else: + db.session.commit() + flask.flash('Copr was deleted successfully.') + return flask.redirect(flask.url_for('coprs_ns.coprs_by_owner', username=username)) + else: + return flask.render_template('coprs/detail/delete.html', form=form, copr=copr) diff --git a/coprs_frontend/coprs/whoosheers.py b/coprs_frontend/coprs/whoosheers.py index 45b5ab6..cefae0f 100644 --- a/coprs_frontend/coprs/whoosheers.py +++ b/coprs_frontend/coprs/whoosheers.py @@ -53,3 +53,7 @@ class CoprUserWhoosheer(AbstractWhoosheer): coprname=copr.name, description=copr.description, instructions=copr.instructions) + + @classmethod + def delete_copr(cls, writer, copr): + writer.delete_by_term('copr_id', copr.id) diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py index 691276f..63f3a06 100644 --- a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py +++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py @@ -370,3 +370,44 @@ class TestCoprUpdatePermissions(CoprsTestCase): data = {'copr_builder_1': '2', 'copr_admin_3': '2'}, follow_redirects = True) assert 'Copr permissions were updated' in r.data + +class TestCoprDelete(CoprsTestCase): + def test_delete(self, f_users, f_coprs, f_db): + with self.tc as c: + with c.session_transaction() as s: + s['openid'] = self.u1.openid_name + + self.db.session.add_all([self.u1, self.c1]) + r = c.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), + data = {'verify': 'yes'}, + follow_redirects = True) + assert 'Copr was deleted successfully' in r.data + assert self.models.Action.query.first().id == self.c1.id + assert not self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first() + + def test_copr_delete_does_not_delete_if_verify_filled_wrongly(self, f_users, f_coprs, f_db): + with self.tc as c: + with c.session_transaction() as s: + s['openid'] = self.u1.openid_name + + self.db.session.add_all([self.u1, self.c1]) + r = c.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), + data = {'verify': 'no'}, + follow_redirects = True) + assert 'Copr was deleted successfully' not in r.data + assert not self.models.Action.query.first() + assert self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first() + + def test_non_owner_cant_delete(self, f_users, f_coprs, f_db): + with self.tc as c: + with c.session_transaction() as s: + s['openid'] = self.u2.openid_name + + self.db.session.add_all([self.u1, self.u2, self.c1]) + r = c.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), + data = {'verify': 'yes'}, + follow_redirects = True) + self.db.session.add_all([self.c1]) + assert 'Copr was deleted successfully' not in r.data + assert not self.models.Action.query.first() + assert self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first()

1 0

[copr] master: Only allow creating builds when the copr is not being renamed or otherwise manipulated (f4fbc29)
by bkabrda＠fedoraproject.org 27 Mar '13

27 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit f4fbc295001e114f2af0d8d13bd6f8b9590b3be6 Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Wed Mar 27 10:40:16 2013 +0100 Only allow creating builds when the copr is not being renamed or otherwise manipulated >--------------------------------------------------------------- coprs_frontend/coprs/logic/builds_logic.py | 1 + .../coprs/views/coprs_ns/coprs_builds.py | 22 +++++++++++-------- .../tests/test_logic/test_builds_logic.py | 9 ++++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py index 191e95b..247c01a 100644 --- a/coprs_frontend/coprs/logic/builds_logic.py +++ b/coprs_frontend/coprs/logic/builds_logic.py @@ -57,6 +57,7 @@ class BuildsLogic(object): @classmethod def add(cls, user, pkgs, copr): + coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr) build = models.Build( pkgs=pkgs, copr=copr, diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py index eae3c1d..1f91e49 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -51,16 +51,20 @@ def copr_new_build(username, coprname): return page_not_found('Copr with name {0} does not exist.'.format(coprname)) if form.validate_on_submit() and flask.g.user.can_build_in(copr): - build = builds_logic.BuildsLogic.add(user=flask.g.user, - pkgs=form.pkgs.data.replace('\n', ' '), - copr=copr) # we're checking authorization above for now - if flask.g.user.proven: - build.memory_reqs = form.memory_reqs.data - build.timeout = form.timeout.data + try: + build = builds_logic.BuildsLogic.add(user=flask.g.user, + pkgs=form.pkgs.data.replace('\n', ' '), + copr=copr) # we're checking authorization above for now + if flask.g.user.proven: + build.memory_reqs = form.memory_reqs.data + build.timeout = form.timeout.data + except exceptions.ActionInProgressException as e: + flask.flash('Can\'t build in this Copr, there is an operation in progress: {0}'.format(e.action)) + db.session.rollback() + else: + flask.flash("Build was added") + db.session.commit() - db.session.commit() - - flask.flash("Build was added") return flask.redirect(flask.url_for('coprs_ns.copr_builds', username=username, coprname=copr.name)) else: return copr_add_build(username=username, coprname=coprname, form=form) diff --git a/coprs_frontend/tests/test_logic/test_builds_logic.py b/coprs_frontend/tests/test_logic/test_builds_logic.py index 007f649..2a2468b 100644 --- a/coprs_frontend/tests/test_logic/test_builds_logic.py +++ b/coprs_frontend/tests/test_logic/test_builds_logic.py @@ -1,4 +1,8 @@ +import pytest + +from coprs.exceptions import ActionInProgressException from coprs.logic.builds_logic import BuildsLogic + from tests.coprs_test_case import CoprsTestCase class TestBuildsLogic(CoprsTestCase): @@ -8,3 +12,8 @@ class TestBuildsLogic(CoprsTestCase): b = BuildsLogic.add(self.u2, 'blah blah', self.c2) self.db.session.commit() assert b.chroots == self.mc3.chroot_name + + def test_add_raises_if_copr_has_unfinished_actions(self, f_users, f_coprs, f_actions, f_db): + with pytest.raises(ActionInProgressException): + b = BuildsLogic.add(self.u1, 'blah blah', self.c1) + self.db.session.rollback()

1 0

[copr] master: Refactor checking for unfinished actions into a separate function (we will use it on multiple places) (d94d26a)
by bkabrda＠fedoraproject.org 27 Mar '13

27 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit d94d26ab592eba0e7eee3120aadd7b20a5f37cbc Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Wed Mar 27 10:20:04 2013 +0100 Refactor checking for unfinished actions into a separate function (we will use it on multiple places) >--------------------------------------------------------------- coprs_frontend/coprs/logic/coprs_logic.py | 19 ++++++++++++++----- coprs_frontend/tests/coprs_test_case.py | 2 ++ .../tests/test_logic/test_coprs_logic.py | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py index abc8f0b..29aa6b0 100644 --- a/coprs_frontend/coprs/logic/coprs_logic.py +++ b/coprs_frontend/coprs/logic/coprs_logic.py @@ -9,6 +9,7 @@ from coprs import whoosheers class CoprsLogic(object): """Used for manipulating Coprs. All methods accept user object as a first argument, as this may be needed in future.""" + @classmethod def get(cls, user, username, coprname, **kwargs): with_builds = kwargs.get('with_builds', False) @@ -91,9 +92,7 @@ class CoprsLogic(object): @classmethod def update(cls, user, copr, check_for_duplicates = True): - action = cls.unfinished_actions(user, copr).first() - if action: - raise exceptions.ActionInProgressException('Action in progress on this copr.', action) + cls.raise_if_unfinished_action(user, copr) existing = cls.exists_for_user(copr.owner, copr.name).first() if existing: @@ -125,13 +124,23 @@ class CoprsLogic(object): update({models.Copr.build_count: models.Copr.build_count + 1}) @classmethod - def unfinished_actions(cls, user, copr): + def unfinished_actions_for(cls, user, copr): actions = models.Action.query.filter(models.Action.object_type=='copr').\ filter(models.Action.object_id==copr.id).\ - filter(models.Action.backend_result==0) + filter(models.Action.backend_result==helpers.BackendResultEnum('waiting')) return actions + @classmethod + def raise_if_unfinished_action(cls, user, copr): + """This method raises ActionInProgressException if given copr has an unfinished + action. Returns None otherwise. + """ + unfinished_actions = cls.unfinished_actions_for(user, copr).all() + if unfinished_actions: + raise exceptions.ActionInProgressException('Action in progress on this copr.', unfinished_actions[0]) + + class CoprPermissionsLogic(object): @classmethod def get(cls, user, copr, searched_user): diff --git a/coprs_frontend/tests/coprs_test_case.py b/coprs_frontend/tests/coprs_test_case.py index dcb0cb9..4f6043f 100644 --- a/coprs_frontend/tests/coprs_test_case.py +++ b/coprs_frontend/tests/coprs_test_case.py @@ -100,6 +100,8 @@ class CoprsTestCase(object): @pytest.fixture def f_actions(self): + # if using actions, we need to flush coprs into db, so that we can get their ids + self.f_db() self.a1 = models.Action(action_type=helpers.ActionTypeEnum('rename'), object_type='copr', object_id=self.c1.id, diff --git a/coprs_frontend/tests/test_logic/test_coprs_logic.py b/coprs_frontend/tests/test_logic/test_coprs_logic.py new file mode 100644 index 0000000..c3f365b --- /dev/null +++ b/coprs_frontend/tests/test_logic/test_coprs_logic.py @@ -0,0 +1,14 @@ +import pytest + +from coprs.exceptions import ActionInProgressException +from coprs.helpers import BackendResultEnum +from coprs.logic.coprs_logic import CoprsLogic + +from tests.coprs_test_case import CoprsTestCase + +class TestCoprsLogic(CoprsTestCase): + def test_update_raises_if_copr_has_unfinished_actions(self, f_users, f_coprs, f_actions, f_db): + self.c1.name = 'foo' + with pytest.raises(ActionInProgressException): + CoprsLogic.update(self.u1, self.c1) + self.db.session.rollback()

1 0

[copr] master: Only display information relevant for backend in waiting builds (e5d18a7)
by bkabrda＠fedoraproject.org 27 Mar '13

27 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit e5d18a79048575b520de95a04d88445e74db69f6 Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Thu Mar 21 11:24:06 2013 +0100 Only display information relevant for backend in waiting builds >--------------------------------------------------------------- .../coprs/views/backend_ns/backend_general.py | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/coprs_frontend/coprs/views/backend_ns/backend_general.py b/coprs_frontend/coprs/views/backend_ns/backend_general.py index 27baca6..7be60a2 100644 --- a/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -15,7 +15,7 @@ def waiting_builds(): builds = query[0:10] return flask.jsonify({'builds': [build.to_dict(options = {'copr': {'owner': {}, - '__columns_except__': ['chroots', 'repos', 'build_count'], + '__columns_only__': ['id', 'name'], '__included_ids__': False}, '__included_ids__': False}) for build in builds]})

1 0

[copr] master: Document models, partially fixes #52 (6361440)
by bkabrda＠fedoraproject.org 21 Mar '13

21 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit 6361440f2f0265c1369374fb457644f98c5c41dc Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Thu Mar 21 11:02:57 2013 +0100 Document models, partially fixes #52 >--------------------------------------------------------------- coprs_frontend/coprs/models.py | 78 ++++++++++++++++++++++++++++++++++------ 1 files changed, 67 insertions(+), 11 deletions(-) diff --git a/coprs_frontend/coprs/models.py b/coprs_frontend/coprs/models.py index 291df11..22e8efe 100644 --- a/coprs_frontend/coprs/models.py +++ b/coprs_frontend/coprs/models.py @@ -55,21 +55,30 @@ class Serializer(object): return map(lambda x: x.name, self.__table__.columns) class User(db.Model, Serializer): + """Represents user of the copr frontend""" id = db.Column(db.Integer, primary_key = True) + # openid_name for fas, e.g. http://bkabrda.id.fedoraproject.org/ openid_name = db.Column(db.String(100), nullable = False) + # just mail :) mail = db.Column(db.String(150), nullable = False) + # is this user proven? proven users can modify builder memory and timeout for single builds proven = db.Column(db.Boolean, default = False) + # is this user admin of the system? admin = db.Column(db.Boolean, default = False) + # stuff for the cli interface api_login = db.Column(db.String(40), nullable = False, default = 'abc') api_token = db.Column(db.String(40), nullable = False, default = 'abc') api_token_expiration = db.Column(db.Date, nullable = False, default = datetime.date(2000, 1, 1)) @property def name(self): + """Returns the short username of the user, e.g. bkabrda""" return self.openid_name.replace('.id.fedoraproject.org/', '').replace('http://', '') - def permissions_for_copr(self, copr): # simple caching of permissions for given copr - # we can't put this into class declaration because the class may be shared by multiple threads + def permissions_for_copr(self, copr): + """Get permissions of this user for the given copr. + Caches the permission during one request, so use this if you access them multiple times + """ if not hasattr(self, '_permissions_for_copr'): self._permissions_for_copr = {} if not copr.name in self._permissions_for_copr: @@ -77,6 +86,7 @@ class User(db.Model, Serializer): return self._permissions_for_copr[copr.name] def can_build_in(self, copr): + """Determine if this user can build in the given copr.""" can_build = False if copr.owner == self: can_build = True @@ -86,6 +96,7 @@ class User(db.Model, Serializer): return can_build def can_edit(self, copr): + """Determine if this user can edit the given copr.""" can_edit = False if copr.owner == self: can_edit = True @@ -96,6 +107,11 @@ class User(db.Model, Serializer): @classmethod def openidize_name(cls, name): + """Creates proper openid_name from short name. + + >>> user.openid_name == User.openidize_name(user.name) + True + """ return 'http://{0}.id.fedoraproject.org/'.format(name) @property @@ -105,14 +121,21 @@ class User(db.Model, Serializer): @property def coprs_count(self): + """Get number of coprs for this user.""" return Copr.query.filter_by(owner=self).count() class Copr(db.Model, Serializer): + """Represents a single copr (private repo with builds, mock chroots, etc.).""" id = db.Column(db.Integer, primary_key = True) + # name of the copr, no fancy chars (checked by forms) name = db.Column(db.String(100), nullable = False) + # string containing urls of additional repos (separated by space) + # that this copr will pull dependencies from repos = db.Column(db.Text) + # time of creation as returned by int(time.time()) created_on = db.Column(db.Integer) + # description and instructions given by copr owner description = db.Column(db.Text) instructions = db.Column(db.Text) # duplicate information, but speeds up a lot and makes queries simpler @@ -129,6 +152,7 @@ class Copr(db.Model, Serializer): @property def repos_list(self): + """Returns repos of this copr as a list of strings""" return self.repos.split() @property @@ -141,12 +165,15 @@ class Copr(db.Model, Serializer): @property def active_mock_chroots(self): + """Returns list of active mock_chroots of this copr""" return filter(lambda x: x.is_active, self.mock_chroots) class CoprPermission(db.Model, Serializer): - # 0 = nothing, 1 = asked for, 2 = approved - # not using enum, as that translates to varchar on some DBs + """Association class for Copr<->Permission relation""" + ## see helpers.PermissionEnum for possible values of the fields below + # can this user build in the copr? copr_builder = db.Column(db.SmallInteger, default = 0) + # can this user serve as an admin? (-> edit and approve permissions) copr_admin = db.Column(db.SmallInteger, default = 0) # relations @@ -156,19 +183,31 @@ class CoprPermission(db.Model, Serializer): copr = db.relationship('Copr', backref = db.backref('copr_permissions')) class Build(db.Model, Serializer): + """Representation of one build in one copr""" id = db.Column(db.Integer, primary_key = True) + # list of space separated urls of packages to build pkgs = db.Column(db.Text) + # was this build canceled by user? canceled = db.Column(db.Boolean, default = False) - # These two are present for every build, as they might change in Copr - # between submitting and starting build => we want to keep them as submitted + ## These two are present for every build, as they might change in Copr + ## between submitting and starting build => we want to keep them as submitted + # list of space separated mock chroot names chroots = db.Column(db.Text, nullable = False) + # list of space separated additional repos repos = db.Column(db.Text) + ## the three below represent time of important events for this build + ## as returned by int(time.time()) submitted_on = db.Column(db.Integer, nullable = False) started_on = db.Column(db.Integer) ended_on = db.Column(db.Integer) + # url of the build results results = db.Column(db.Text) + # status as returned by backend, see build.state for value explanation + # (TODO: this would deserve an enum) status = db.Column(db.Integer) + # memory requirements for backend builder memory_reqs = db.Column(db.Integer, default = constants.DEFAULT_BUILD_MEMORY) + # maximum allowed time of build, build will fail if exceeded timeout = db.Column(db.Integer, default = constants.DEFAULT_BUILD_TIMEOUT) # relations @@ -179,6 +218,7 @@ class Build(db.Model, Serializer): @property def state(self): + """Return text representation of status of this build""" if self.status == 1: return 'succeeded' elif self.status == 0: @@ -191,17 +231,26 @@ class Build(db.Model, Serializer): @property def cancelable(self): + """Find out if this build is cancelable. + + ATM, build is cancelable only if it wasn't grabbed by backend. + """ return self.state == 'pending' class MockChroot(db.Model, Serializer): + """Representation of mock chroot""" id = db.Column(db.Integer, primary_key = True) - os_release = db.Column(db.String(50), nullable = False) # fedora/epel/... - os_version = db.Column(db.String(50), nullable = False) # 18/rawhide/... - arch = db.Column(db.String(50), nullable = False) # x86_64/i686/... + # fedora/epel/..., mandatory + os_release = db.Column(db.String(50), nullable = False) + # 18/rawhide/..., optional (mock chroot doesn't need to have this) + os_version = db.Column(db.String(50), nullable = False) + # x86_64/i686/..., mandatory + arch = db.Column(db.String(50), nullable = False) is_active = db.Column(db.Boolean, default = True) @property def chroot_name(self): + """Textual representation of name of this chroot""" if self.os_version: format_string = '{rel}-{ver}-{arch}' else: @@ -211,6 +260,7 @@ class MockChroot(db.Model, Serializer): arch=self.arch) class CoprChroot(db.Model, Serializer): + """Representation of Copr<->MockChroot relation""" mock_chroot_id = db.Column(db.Integer, db.ForeignKey('mock_chroot.id'), primary_key = True) mock_chroot = db.relationship('MockChroot', backref = db.backref('copr_chroots')) copr_id = db.Column(db.Integer, db.ForeignKey('copr.id'), primary_key = True) @@ -219,16 +269,22 @@ class CoprChroot(db.Model, Serializer): cascade='all,delete,delete-orphan')) class Action(db.Model, Serializer): + """Representation of a custom action that needs backends cooperation""" id = db.Column(db.Integer, primary_key=True) - # delete, rename, + # delete, rename, ...; see helpers.ActionTypeEnum action_type = db.Column(db.Integer, nullable=False) - # copr, + # copr, ...; downcase name of class of modified object object_type = db.Column(db.String(20)) + # id of the modified object object_id = db.Column(db.Integer) + # old and new values of the changed property old_value = db.Column(db.String(255)) new_value = db.Column(db.String(255)) + # backend result, see helpers.BackendResultEnum backend_result = db.Column(db.Integer, default=helpers.BackendResultEnum('waiting')) + # optional message from the backend backend_message = db.Column(db.Text) + # time created as returned by int(time.time()) created_on = db.Column(db.Integer) def __str__(self):

1 0

[copr] master: Properly deal with inactive mock chroots, closes #50 (2fdf8a7)
by bkabrda＠fedoraproject.org 21 Mar '13

21 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git On branch : master >--------------------------------------------------------------- commit 2fdf8a7328687f59a4a2f508fc775202f46dbe42 Author: Bohuslav Kabrda <bkabrda(a)redhat.com> Date: Thu Mar 21 10:27:42 2013 +0100 Properly deal with inactive mock chroots, closes #50 >--------------------------------------------------------------- coprs_frontend/coprs/logic/builds_logic.py | 4 +--- coprs_frontend/coprs/logic/coprs_logic.py | 11 +++++++++-- coprs_frontend/coprs/models.py | 4 ++++ .../coprs/templates/coprs/detail/overview.html | 12 ++++++------ coprs_frontend/coprs/templates/coprs/show.html | 2 +- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py index 1ef7252..191e95b 100644 --- a/coprs_frontend/coprs/logic/builds_logic.py +++ b/coprs_frontend/coprs/logic/builds_logic.py @@ -61,9 +61,7 @@ class BuildsLogic(object): pkgs=pkgs, copr=copr, repos=copr.repos, - chroots=' '.join(map(lambda x: x.chroot_name, - filter(lambda b: b.is_active, #only add active chroots - copr.mock_chroots))), + chroots=' '.join(map(lambda x: x.chroot_name, copr.active_mock_chroots)), user=user, submitted_on=int(time.time())) # no need to check for authorization here diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py index 9418090..abc8f0b 100644 --- a/coprs_frontend/coprs/logic/coprs_logic.py +++ b/coprs_frontend/coprs/logic/coprs_logic.py @@ -41,7 +41,9 @@ class CoprsLogic(object): query = db.session.query(models.Copr).\ join(models.Copr.owner).\ - options(db.contains_eager(models.Copr.owner)) + options(db.contains_eager(models.Copr.owner)).\ + order_by(models.Copr.id.desc()) + if user_relation == 'owned': query = query.filter(models.User.openid_name == models.User.openidize_name(username)) elif user_relation == 'allowed': @@ -51,7 +53,12 @@ class CoprsLogic(object): join(aliased_user, models.CoprPermission.user).\ filter(aliased_user.openid_name == models.User.openidize_name(username)) if with_mock_chroots: - query = query.options(db.subqueryload(*models.Copr.mock_chroots.attr)) + query = query.outerjoin(*models.Copr.mock_chroots.attr).\ + options(db.contains_eager(*models.Copr.mock_chroots.attr)).\ + order_by(models.MockChroot.os_release.asc()).\ + order_by(models.MockChroot.os_version.asc()).\ + order_by(models.MockChroot.arch.asc()) + return query @classmethod diff --git a/coprs_frontend/coprs/models.py b/coprs_frontend/coprs/models.py index 957b80e..291df11 100644 --- a/coprs_frontend/coprs/models.py +++ b/coprs_frontend/coprs/models.py @@ -139,6 +139,10 @@ class Copr(db.Model, Serializer): def instructions_or_not_filled(self): return self.instructions or 'Instructions not filled in by author.' + @property + def active_mock_chroots(self): + return filter(lambda x: x.is_active, self.mock_chroots) + class CoprPermission(db.Model, Serializer): # 0 = nothing, 1 = asked for, 2 = approved # not using enum, as that translates to varchar on some DBs diff --git a/coprs_frontend/coprs/templates/coprs/detail/overview.html b/coprs_frontend/coprs/templates/coprs/detail/overview.html index 0d74561..6152ddf 100644 --- a/coprs_frontend/coprs/templates/coprs/detail/overview.html +++ b/coprs_frontend/coprs/templates/coprs/detail/overview.html @@ -12,10 +12,10 @@ <th class="leftmost">Release</th> <th class="rightmost">Architecture</th> </tr> - {% for mock_chroot in copr.mock_chroots %} - {% if loop.index < copr.mock_chroots|length %} - {% if mock_chroot.os_release != copr.mock_chroots[loop.index].os_release or - mock_chroot.os_version != copr.mock_chroots[loop.index].os_version %} + {% for mock_chroot in copr.active_mock_chroots %} + {% if loop.index < copr.active_mock_chroots|length %} + {% if mock_chroot.os_release != copr.active_mock_chroots[loop.index].os_release or + mock_chroot.os_version != copr.active_mock_chroots[loop.index].os_version %} {# next release is different => release-end #} <tr class="release-end"> {% else %} @@ -24,8 +24,8 @@ {% else %}{# last line => release-end for sure #} <tr class="release-end"> {% endif %} - {% if mock_chroot.os_release != copr.mock_chroots[loop.index0 - 1].os_release or - mock_chroot.os_version != copr.mock_chroots[loop.index0 - 1].os_version or + {% if mock_chroot.os_release != copr.active_mock_chroots[loop.index0 - 1].os_release or + mock_chroot.os_version != copr.active_mock_chroots[loop.index0 - 1].os_version or loop.index0 == 0 %} {# previous os_release-os_version were different or this is the first one #} <td>{{ mock_chroot.os_release|capitalize }} {{ mock_chroot.os_version }}</td> diff --git a/coprs_frontend/coprs/templates/coprs/show.html b/coprs_frontend/coprs/templates/coprs/show.html index c28ebcc..5ccde32 100644 --- a/coprs_frontend/coprs/templates/coprs/show.html +++ b/coprs_frontend/coprs/templates/coprs/show.html @@ -25,7 +25,7 @@ <a href="{{ url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name) }}">{{ copr.owner.name }}/{{ copr.name }}</a> <p>{{ copr.description_or_not_filled }}</p> <p class="repos"> - {% for mock_chroot in copr.mock_chroots %} + {% for mock_chroot in copr.active_mock_chroots %} {{ mock_chroot.os_release|os_name_short(mock_chroot.os_version) }}.{{ mock_chroot.arch }}{% if not loop.last %}, {% endif %} {% endfor %} </p>

1 0

[copr] branch 'devel' deleted
by Pierre-YvesChibon 20 Mar '13

20 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git Deleted branch: devel

2 1

[copr] master's head updated: Add a api_login field (0da3d25)
by Pierre-YvesChibon 20 Mar '13

20 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git Branch 'master' now includes: 0da3d25 Add a api_login field

1 0

[copr] branch 'devel' created
by Pierre-YvesChibon 20 Mar '13

20 Mar '13

Repository : http://git.fedorahosted.org/cgit/copr.git New branch : devel Referencing: 0da3d25cf605280de9794ed6f2a5bd2a499a9863

2 1

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

copr-devel March 2013