PR#680: [frontend] allow user to remove outdated chroot; fix #624 - copr/copr

copr / copr

#680 [frontend] allow user to remove outdated chroot; fix #624

Closed 5 years ago by praiskup. Opened 5 years ago by frostyx.

copr/ frostyx/copr remove-outdated-chroot-button into master

[frontend] remove a redundant condition for outdated chroots

Jakub Kadlčík • 5 years ago

7d93ef7

[frontend] allow user to remove outdated chroot; fix #624

Jakub Kadlčík • 5 years ago

da6faff

frontend/coprs_frontend/coprs/forms.py

file modified

+2 -1

		`@@ -965,7 +965,8 @@`


		`class CoprChrootExtend(FlaskForm):`
		`- name = wtforms.StringField("Chroot name", validators=[wtforms.validators.DataRequired])`
		`+ extend = wtforms.StringField("Chroot name")`
		`+ expire = wtforms.StringField("Chroot name")`


		`class CoprLegalFlagForm(FlaskForm):`

frontend/coprs_frontend/coprs/models.py

file modified

+2 -1

		`@@ -1222,7 +1222,8 @@`
		`if not self.delete_after:`
		`return None`
		`now = datetime.datetime.now()`
		`- return (self.delete_after - now).days`
		`+ days = (self.delete_after - now).days`
		`+ return days if days > 0 else 0`

		`def to_dict(self):`
		`options = {"__columns_only__": [`

frontend/coprs_frontend/coprs/templates/coprs/detail/settings/repositories.html

file modified

+10 -3

		`@@ -24,15 +24,22 @@`
		`</p>`
		`<form action="" method="POST">`
		`<table class="table table-bordered">`
		`- <thead><tr><th>Release</th><th>Architecture</th><th>Remaining time</th><th>Extend</th></tr></thead>`
		`+ <thead><tr><th>Release</th><th>Architecture</th><th>Remaining time</th><th>Action</th></tr></thead>`
		`<tbody>`
		`{% for chroot in outdated_chroots %}`
		`<tr>`
		`<td>{{ chroot.mock_chroot.os.capitalize() }}</td>`
		`<td>{{ chroot.mock_chroot.arch }}</td>`
		`- <td>{% if chroot.delete_after_days %}{{ chroot.delete_after_days }} days{% else %} Unknown {% endif %}</td>`
		`<td>`
		`- <button name="name" class="btn btn-default" type="submit" value="{{ chroot.mock_chroot.name }}">Extend</button>`
		`+ {% if not chroot.delete_after_days %}`
		`+ To be removed in next cleanup`
		`+ {% else %}`
		`+ {{ chroot.delete_after_days }} days`
		`+ {% endif %}`
		`+ </td>`
		`+ <td>`
		`+ <button name="extend" class="btn btn-primary" type="submit" value="{{ chroot.mock_chroot.name }}">Extend</button>`
		`+ <button name="expire" class="btn btn-danger" type="submit" value="{{ chroot.mock_chroot.name }}">Expire now</button>`
		`</td>`
		`</tr>`
		`</tbody>`

frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py

file modified

+15 -2

		`@@ -599,8 +599,21 @@`
		`return flask.redirect(url_for_copr_details(copr))`

		`form = forms.CoprChrootExtend()`
		`- copr_chroot = coprs_logic.CoprChrootsLogic.get_by_name(copr, form.name.data, active_only=False).one()`
		`- delete_after_days = app.config["DELETE_EOL_CHROOTS_AFTER"] + 1`
		`+ if form.extend.data:`
		`+ delete_after_days = app.config["DELETE_EOL_CHROOTS_AFTER"] + 1`
		`+ chroot_name = form.extend.data`
		`+ flask.flash("Repository for {} will be preserved for another {} days from now"`
		`+ .format(chroot_name, app.config["DELETE_EOL_CHROOTS_AFTER"]))`
		`+ elif form.expire.data:`
		`+ delete_after_days = 0`
		`+ chroot_name = form.expire.data`
		`+ flask.flash("Repository for {} is scheduled to be removed."`
		+ "If you changed your mind, click 'Extend` to revert your decision."
		`+ .format(chroot_name))`
		`+ else:`
		`+ raise ValidationError("Copr chroot needs to be either extended or expired")`
		`+`
		`+ copr_chroot = coprs_logic.CoprChrootsLogic.get_by_name(copr, chroot_name, active_only=False).one()`
		`delete_after_timestamp = datetime.datetime.now() + datetime.timedelta(days=delete_after_days)`
		`coprs_logic.CoprChrootsLogic.update_chroot(flask.g.user, copr_chroot,`
		`delete_after=delete_after_timestamp)`

frostyx commented 5 years ago

Fix #624

I am adding an "Expire" button next to the "Extend" button. If a
user clicks to expire the chroot, it will be removed on our next
cron-scheduled cleanup. Even if the removal is delayed for some
reason, user is not notified about the chroot anymore.

praiskup commented 5 years ago

Double referenced id (nit)

    [frontend] allow user to remove outdated chroot; fix #624

    Fix #624

praiskup commented on line 16 of frontend/coprs_frontend/coprs/templates/coprs/detail/settings/repositories.html 5 years ago

Interesting, this is not new thing -- but it still raises question "when this might happen", could we add a ?

Edited 5 years ago by praiskup

praiskup commented 5 years ago

Very nice, thank you!

praiskup commented 5 years ago

frostyx commented on line 16 of frontend/coprs_frontend/coprs/templates/coprs/detail/settings/repositories.html 5 years ago

"when this might happen" I was asking the same thing, but rather left it there. However, I think, that we can get rid of it. Here, in this template, we are iterating over copr.outdated_chroots, which is [... for chroot in ... if chroot.delete_after] and chroot.delete_after_days can be None only when chroot.delete_after is None; i.e. this might not happen.

1 new commit added

[frontend] remove a redundant condition for outdated chroots

5 years ago

frostyx commented 5 years ago

"when this might happen" I was asking the same thing, but rather left it there. However, I think, that we can get rid of it.

Done. In separate commit though, so we can easily revert it if something goes wrong.

msuchy commented 5 years ago

The "Expire" button seems clumsy to me. I would have doubts about what it actually does. What about "Expire now"? With flask.flash("Repository {} will be in few hours. If you change your mind, you can click 'Extend` to revert your decision.").

rebased onto 00a7ba76254ba1e611b29d4da56d897ed969b153

5 years ago

frostyx commented 5 years ago

The "Expire" button seems clumsy to me. I would have doubts about what it actually does. What about "Expire now"

Fixed

With flask.flash(...)

I've added it, and also added flask.flash for "Extend" button

praiskup commented 5 years ago

Repository {} will be in few hours. If you change your mind, you can click 'Extend` to revert your decision."

Is it non-racy statement? :-) I mean, what if user hits the button right before the cron job is in action?

praiskup commented on line 13 of frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py 5 years ago

I'm curious whether - to comply with the flash message - we should guarantee a few hours here...

frostyx commented 5 years ago

I have changed the wording to

Repository for {} is scheduled to be removed. If you changed your mind, click ...

rebased onto da6faff

5 years ago

praiskup commented 5 years ago

$ git diff 00a7ba7..da6faff
diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index c2ed2581..3d3d0aa2 100644
--- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -607,8 +607,8 @@ def copr_repositories_post(copr):
     elif form.expire.data:
         delete_after_days = 0
         chroot_name = form.expire.data
-        flask.flash("Repository for {} will be removed in few hours."
-                    "If you change your mind, click 'Extend` to revert your decision."
+        flask.flash("Repository for {} is scheduled to be removed."
+                    "If you changed your mind, click 'Extend` to revert your decision."
                     .format(chroot_name))
     else:
         raise ValidationError("Copr chroot needs to be either extended or expired")

praiskup commented 5 years ago

Merging... (btw., the user experience with "Expire" is just awesome now).

praiskup commented 5 years ago

Merged as 903b7fe