PR#1480: Input field for setting timeout for builds - copr/copr

		`@@ -0,0 +1,21 @@`
		`+ Name: test-timeout`
		`+ Version: 1.0`
		`+ Release: 1%{?dist}`
		`+ Summary: Testing spec file for runtest-timeout.sh`
		`+ License: GPLv2`
		`+ URL: https://pagure.io/copr/copr`
		`+`
		`+ %description`
		`+ Test timeout`
		`+`
		`+ %files`
		`+`
		`+ %build`
		`+ while :`
		`+ do`
		`+ sleep 1`
		`+ done`
		`+`
		`+ %changelog`
		`+ * Fri Sep 11 2020 Silvie Chlupova <schlupov@redhat.com> 1.0-1`
		`+ - Initial version`

beaker-tests/Sanity/copr-cli-basic-operations/runtest-timeout.sh

file added

+65

		`@@ -0,0 +1,65 @@`
		`+ #!/bin/bash`
		`+ # vim: dict=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+ #`
		`+ # runtest.sh of /tools/copr/Sanity/copr-cli-basic-operations`
		`+ # Description: Tests basic operations of copr using copr-cli.`
		`+ # Author: Silvie Chlupova <schlupov@redhat.com>`
		`+ #`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+ #`
		`+ # Copyright (c) 2014 Red Hat, Inc.`
		`+ #`
		`+ # This program is free software: you can redistribute it and/or`
		`+ # modify it under the terms of the GNU General Public License as`
		`+ # published by the Free Software Foundation, either version 2 of`
		`+ # the License, or (at your option) any later version.`
		`+ #`
		`+ # This program is distributed in the hope that it will be`
		`+ # useful, but WITHOUT ANY WARRANTY; without even the implied`
		`+ # warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR`
		`+ # PURPOSE. See the GNU General Public License for more details.`
		`+ #`
		`+ # You should have received a copy of the GNU General Public License`
		`+ # along with this program. If not, see http://www.gnu.org/licenses/.`
		`+ #`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+`
		`+ # Include Beaker environment`
		`+ . /usr/bin/rhts-environment.sh \|\| exit 1`
		`+ . /usr/share/beakerlib/beakerlib.sh \|\| exit 1`
		`+`
		`+ # Load config settings`
		`+ HERE=$(dirname "$(realpath "$0")")`
		`+ source "$HERE/config"`
		`+ source "$HERE/helpers"`
		`+`
		`+ rlJournalStart`
		`+ rlPhaseStartSetup`
		`+ rlAssertRpm "copr-cli"`
		`+ rlAssertRpm "jq"`
		`+ rlAssertExists ~/.config/copr`
		`+ rlPhaseEnd`
		`+`
		`+ rlPhaseStartTest`
		`+ rlRun "copr-cli create --chroot $CHROOT ${NAME_PREFIX}Timeout"`
		`+ rlRun -s "copr-cli build --timeout 72500 ${NAME_PREFIX}Timeout $HELLO"`
		`+ rlRun "parse_build_id"`
		`+ rlRun "copr watch-build $BUILD_ID"`
		`+ rlRun "curl $FRONTEND_URL/backend/get-build-task/$BUILD_ID-$CHROOT \| jq .timeout"`
		`+ rlRun "BUILD_TIMEOUT=$(curl "$FRONTEND_URL"/backend/get-build-task/"$BUILD_ID"-"$CHROOT" \| jq .timeout)"`
		`+ rlAssertEquals "Test that timeout is set to 72500" "$BUILD_TIMEOUT" 72500`
		`+`
		`+ rlRun "SRPM=$(rpmbuild -bs "$HERE"/files/test-timeout.spec \|grep Wrote: \|cut -d ' ' -f2)"`
		`+ rlRun -s "copr-cli build --timeout 10 ${NAME_PREFIX}Timeout $SRPM"`
		`+ rlRun "parse_build_id"`
		`+ rlRun "copr watch-build $BUILD_ID"`
		`+ rlRun "LOG=$(curl "$BACKEND_URL/results/${NAME_PREFIX}Timeout/$CHROOT/00$BUILD_ID-test-timeout/builder-live.log.gz" \| gunzip)"`
		+ rlAssertEquals "timeout in log" `echo $LOG \| grep 'sending INT'` 0
		`+ rlPhaseEnd`
		`+`
		`+ rlPhaseStartCleanup`
		`+ cleanProject "${NAME_PREFIX}Timeout"`
		`+ rlPhaseEnd`
		`+ rlJournalPrintText`
		`+ rlJournalEnd`

cli/man/copr-cli.1.asciidoc

file modified

+1 -1

		`@@ -203,7 +203,7 @@`
		`Override memory for this build. This is actually not used and it have no effect.`

		`--timeout::`
		`- Override timeout for this build. This is actually not used and it have no effect.`
		`+ Override timeout for this build.`

		`--nowait::`
		`Don't wait for build completion.`

docker/frontend/files/etc/copr/copr.conf

file modified

+2 -2

		`@@ -143,6 +143,6 @@`
		`MIN_BUILD_MEMORY = 2048`
		`MAX_BUILD_MEMORY = 4096`
		`# in seconds`
		`- DEFAULT_BUILD_TIMEOUT = 3600 * 24`
		`+ DEFAULT_BUILD_TIMEOUT = 3600 * 5`
		`MIN_BUILD_TIMEOUT = 0`
		`- MAX_BUILD_TIMEOUT = 86400`
		`+ MAX_BUILD_TIMEOUT = 108000`

frontend/coprs_frontend/config/copr.conf

file modified

+3 -1

		`@@ -128,4 +128,6 @@`
		`PAGES_URLS_COUNT = 5`

		`# in seconds. After this time, copr-rpmbuild stops the build`
		`- DEFAULT_BUILD_TIMEOUT = 3600 * 24`
		`+ DEFAULT_BUILD_TIMEOUT = 3600 * 5`
		`+ MIN_BUILD_TIMEOUT = 0`
		`+ MAX_BUILD_TIMEOUT = 108000`

frontend/coprs_frontend/coprs/config.py

file modified

+2 -2

		`@@ -108,9 +108,9 @@`
		`MIN_BUILD_MEMORY = 2048`
		`MAX_BUILD_MEMORY = 4096`
		`# in seconds`
		`- DEFAULT_BUILD_TIMEOUT = 3600 * 24`
		`+ DEFAULT_BUILD_TIMEOUT = 3600 * 5`
		`MIN_BUILD_TIMEOUT = 0`
		`- MAX_BUILD_TIMEOUT = 86400`
		`+ MAX_BUILD_TIMEOUT = 108000`

		`class ProductionConfig(Config):`
		`DEBUG = False`

frontend/coprs_frontend/coprs/forms.py

file modified

+9 -17

		`@@ -502,6 +502,11 @@`
		`class APICoprDeleteForm(CoprDeleteForm):`
		`verify = wtforms.BooleanField("Confirm deleting", false_values=FALSE_VALUES)`

		`+ def seconds_to_pretty_hours(sec):`
		`+ minutes = round(sec / 60)`
		`+ hours = minutes // 60`
		`+ minutes = minutes % 60`
		`+ return hours if not minutes else "{}:{:02d}".format(hours, minutes)`

		`# @TODO jkadlcik - rewrite via BaseBuildFormFactory after fe-dev-cloud is back online`
		`class BuildFormRebuildFactory(object):`
		`@@ -516,16 +521,10 @@`
		`selected.append(ch)`
		`return selected`

		`- memory_reqs = wtforms.IntegerField(`
		`- "Memory requirements",`
		`- validators=[`
		`- wtforms.validators.NumberRange(`
		`- min=app.config["MIN_BUILD_MEMORY"],`
		`- max=app.config["MAX_BUILD_MEMORY"])],`
		`- default=app.config["DEFAULT_BUILD_MEMORY"])`
		`-`
		`timeout = wtforms.IntegerField(`
		`"Timeout",`
		`+ description="Optional - number of seconds we allow the builds to run, default is {0} ({1}h)".format(`
		`+ app.config["DEFAULT_BUILD_TIMEOUT"], seconds_to_pretty_hours(app.config["DEFAULT_BUILD_TIMEOUT"])),`
		`validators=[`
		`wtforms.validators.NumberRange(`
		`min=app.config["MIN_BUILD_TIMEOUT"],`
		`@@ -1033,17 +1032,10 @@`
		`selected.append(ch)`
		`return selected`

		`- F.memory_reqs = wtforms.IntegerField(`
		`- "Memory requirements",`
		`- validators=[`
		`- wtforms.validators.Optional(),`
		`- wtforms.validators.NumberRange(`
		`- min=app.config["MIN_BUILD_MEMORY"],`
		`- max=app.config["MAX_BUILD_MEMORY"])],`
		`- default=app.config["DEFAULT_BUILD_MEMORY"])`
		`-`
		`F.timeout = wtforms.IntegerField(`
		`"Timeout",`
		`+ description="Optional - number of seconds we allow the builds to run, default is {0} ({1}h)".format(`
		`+ app.config["DEFAULT_BUILD_TIMEOUT"], seconds_to_pretty_hours(app.config["DEFAULT_BUILD_TIMEOUT"])),`
		`validators=[`
		`wtforms.validators.Optional(),`
		`wtforms.validators.NumberRange(`

frontend/coprs_frontend/coprs/logic/builds_logic.py

file modified

+2 -3

		`@@ -600,9 +600,8 @@`
		`copr_dirname=copr_dirname,`
		`)`

		`- if user.proven:`
		`- if "timeout" in build_options:`
		`- build.timeout = build_options["timeout"]`
		`+ if "timeout" in build_options:`
		`+ build.timeout = build_options["timeout"]`

		`return build`

frontend/coprs_frontend/coprs/templates/_helpers.html

file modified

+11 -7

		`@@ -568,13 +568,17 @@`
		`</div>`
		`</div>`
		`{{ form.csrf_token }}`
		`- {% if g.user.proven %}`
		`- {{ render_field(form.memory_reqs) }}`
		`- {{ render_field(form.timeout) }}`
		`- {% else %} {# once we pass the hidden attribute, the field will just be hidden, it seems #}`
		`- {{ render_field(form.memory_reqs, hidden = True) }}`
		`- {{ render_field(form.timeout, hidden = True) }}`
		`- {% endif %}`
		`+ <div class="form-group">`
		`+ <label class="col-sm-2 control-label" for="textInput-markup" style="text-align: left">`
		`+ Timeout`
		`+ </label>`
		`+ <div class="col-sm-6">`
		`+ <input id="timeout" class="form-control" name="timeout" type="text" value="{{ form.timeout.default }}" >`
		`+ <li class="help-block">`
		`+ <small class="text-muted pficon pficon-info"></small> {{form.timeout.description\|safe}}`
		`+ </li>`
		`+ </div>`
		`+ </div>`
		`<div class="form-group">`
		`<label class="col-sm-2 control-label" for="textInput-markup" style="text-align: left">`
		`Other Options`

frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py

file modified

		`@@ -139,6 +139,7 @@`
		`chroot_names=form.selected_chroots,`
		`background=form.background.data,`
		`copr_dirname=form.project_dirname.data,`
		`+ timeout=form.timeout.data,`
		`) for pkg in pkgs]`
		`return process_creating_new_build(copr, form, create_new_build)`

		`@@ -159,6 +160,7 @@`
		`chroot_names=form.selected_chroots,`
		`background=form.background.data,`
		`copr_dirname=form.project_dirname.data,`
		`+ timeout=form.timeout.data,`
		`)`
		`return process_creating_new_build(copr, form, create_new_build)`

		`@@ -183,6 +185,7 @@`
		`chroot_names=form.selected_chroots,`
		`background=form.background.data,`
		`copr_dirname=form.project_dirname.data,`
		`+ timeout=form.timeout.data,`
		`)`
		`return process_creating_new_build(copr, form, create_new_build)`

		`@@ -233,6 +236,7 @@`
		`form.selected_chroots,`
		`background=form.background.data,`
		`copr_dirname=form.project_dirname.data,`
		`+ timeout=form.timeout.data,`
		`)`
		`return process_creating_new_build(copr, form, create_new_build)`

		`@@ -252,6 +256,7 @@`
		`form.selected_chroots,`
		`background=form.background.data,`
		`copr_dirname=form.project_dirname.data,`
		`+ timeout=form.timeout.data,`
		`)`
		`return process_creating_new_build(copr, form, create_new_build)`

		`@@ -274,6 +279,7 @@`
		`chroot_names=form.selected_chroots,`
		`background=form.background.data,`
		`copr_dirname=form.project_dirname.data,`
		`+ timeout=form.timeout.data,`
		`)`
		`return process_creating_new_build(copr, form, create_new_build)`

schlupov commented 2 years ago

Fixes: #1303

Metadata Update from @schlupov:
- Pull-request tagged with: wip

2 years ago

rebased onto 3cc17a550e229ad71e798546f56c0d201be58a15

2 years ago

1 new commit added

cli: fix timeout option to allow change timeout for build

2 years ago

4 new commits added

cli: fix timeout option to allow change timeout for build
frontend: reduce the timeout to 5 hours
frontend: input field for setting timeout for builds
frontend: remove setting memory requirements

2 years ago

4 new commits added

cli: fix timeout option to allow change timeout for build
frontend: reduce the timeout to 5 hours
frontend: input field for setting timeout for builds
frontend: remove setting memory requirements

2 years ago

4 new commits added

cli: fix timeout option to allow change timeout for build
frontend: reduce the timeout to 5 hours
frontend: input field for setting timeout for builds
frontend: remove setting memory requirements

2 years ago

schlupov commented 2 years ago

There are 833754 builds (I'm using database dump from 26.8.2020)
select count(*) from build;

4535 builds took at least 10 hours
select count(distinct build_id) from build_chroot where (ended_on-started_on)>=10*3600;

8371 builds took at least 5 hours (new DEFAULT_BUILD_TIMEOUT)
select count(distinct build_id) from build_chroot where (ended_on-started_on)>=5*3600;
so only 1% of builds need more time than the default.

The average time required for a build chroot is 30 minutes
select avg((ended_on-started_on)/60) as average from build_chroot;

The new MAX_BUILD_TIMEOUT is set to 24 hours.

Metadata Update from @schlupov:
- Pull-request untagged with: wip

2 years ago

praiskup commented 2 years ago

Seems OK. Btw., all the values are configurable and I believe we have 30h as the max build timeout in Fedora Copr (needed by full chromium build).

praiskup commented 2 years ago

Slight request for enhancement; can you please either replace the default
value in form with

Optional - number of seconds we allow the builds to run, default is ...

(you can set "render_kw={"placeholder":...)

Or add a simlar field.description?

4 new commits added

cli: fix timeout option to allow change timeout for build
frontend: reduce the timeout to 5 hours
frontend: input field for setting timeout for builds
frontend: remove setting memory requirements

2 years ago

schlupov commented 2 years ago

this is what it looks like now

praiskup commented 2 years ago

praiskup commented on line 36 of frontend/coprs_frontend/coprs/forms.py 2 years ago

NIt, the seconds are configurable, but (5h) not.

4 new commits added

cli: fix timeout option to allow change timeout for build
frontend: reduce the timeout to 5 hours
frontend: input field for setting timeout for builds
frontend: remove setting memory requirements

2 years ago

schlupov commented 2 years ago

I removed that (5h), I didn't realize it can be misleading.

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

2 years ago

frostyx commented 2 years ago

I removed that (5h), I didn't realize it can be misleading.

Looking at the screenshot, the (5h) was helpful for me. It's hard to imagine what 18000 seconds are in human time. I would vote for putting the number back, but instead of hardcoding 5h there, calculating the number instead. What do you think?

rebased onto 12db8c1b64c3f121cf9ba7004b246859f83ab496

2 years ago

praiskup commented on line 52 of beaker-tests/Sanity/copr-cli-basic-operations/runtest-timeout.sh 2 years ago

Nice. Can you please add a check that the build timeout is respected? Some never-ending process + short timeout?

praiskup commented on line 12 of beaker-tests/Sanity/copr-cli-basic-operations/runtest-timeout.sh 2 years ago

c&p bug

5 new commits added

beaker-tests: set timeout for a build
cli: fix timeout option to allow change timeout for build
frontend: reduce the timeout to 5 hours
frontend: input field for setting timeout for builds
frontend: remove setting memory requirements

2 years ago

schlupov commented 2 years ago

Looking at the screenshot, the (5h) was helpful for me. It's hard to imagine what 18000 seconds are in human time. I would vote for putting the number back, but instead of hardcoding 5h there, calculating the number instead. What do you think?

Good idea, I fixed that.

praiskup commented on line 16 of frontend/coprs_frontend/coprs/forms.py 2 years ago

// 3600 is pretty inaccurate. Can you use something like:

In [53]: def seconds_to_pretty_hours(sec): 
    ...:     sec = round(sec * 100 / 3600) 
    ...:     return "{:g}".format(sec/100)

praiskup commented on line 16 of frontend/coprs_frontend/coprs/forms.py 2 years ago

Or:

In [64]: def seconds_to_pretty_hours(sec): 
    ...:     minutes = round(sec / 60) 
    ...:     hours = minutes // 60 
    ...:     minutes = minutes % 60 
    ...:     if not minutes: 
    ...:         return hours 
    ...:     else: 
    ...:         return "{}:{:02d}".format(hours, minutes)

... or anything similar.

rebased onto fefed5425b492709206ac92a15d707412a020b61

2 years ago

rebased onto 8f98261

2 years ago

praiskup commented 2 years ago

Looks fine to me, thank you. Can you pleaes fix pylint (add the the docstring) and fix testsuite?

praiskup commented 2 years ago

ok... testsuite is Ok actually... merging

Pull-Request has been merged by praiskup

2 years ago

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

2 years ago

praiskup commented 2 years ago

FWIW, since the build [1] we are building on F32, and the copr-frontend
build takes ~30 minutes..., and it is because the fallbacks on the name
resolution takes ages, this is fixed by mock change [2].

Mock 2.6 now got stable karma, so it should be on builders tomorrow.

[1] https://pagure.io/copr/copr/c/20ca854176ed3008e57d08e7e61fb2ab5a79e66a
[2] https://github.com/rpm-software-management/mock/commit/c44571821570e8cb0e65ef5a445313b793992706

Metadata

Assignee

None

Tags

No Tags

copr / copr

Source Code

Documentation

#1480 Input field for setting timeout for builds Merged 2 years ago by praiskup. Opened 2 years ago by schlupov. copr/ schlupov/copr better_timeout into master

Metadata

Flags

Changes Summary 10

#1480 Input field for setting timeout for builds

Merged 2 years ago by praiskup. Opened 2 years ago by schlupov.

copr/ schlupov/copr better_timeout into master