PR#675: [frontend] replace cron configuration automatically - copr/copr

copr / copr

#675 [frontend] replace cron configuration automatically

Merged 5 years ago by praiskup. Opened 5 years ago by praiskup.

Unknown source auto-cron-frontend into master

[frontend] replace cron configuration automatically

Pavel Raiskup • 5 years ago

19f2d47

frontend/conf/cron.daily/copr-frontend

file modified

+5 -1

		`@@ -1,4 +1,8 @@`
		`- #!/usr/bin/sh`
		`+ #! /usr/sh`
		`+`
		`+ # This file contains all the _mandatory_ Copr frontend tasks to be executed`
		`+ # daily. Don't edit this file manually, it is automatically updated with`
		`+ # copr-frontend.rpm.`

		`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`

frontend/conf/cron.hourly/copr-frontend

file modified

+5 -3

		`@@ -1,6 +1,8 @@`
		`- #!/usr/bin/sh`
		`+ #! /bin/sh`
		`+`
		`+ # This file contains all the _mandatory_ Copr frontend tasks to be executed`
		`+ # hourly. Don't edit this file manually, it is automatically updated with`
		`+ # copr-frontend.rpm.`

		`- runuser -c '/usr/share/copr/coprs_frontend/run/check_for_anitya_version_updates.py --backend pypi --delta=172800 &> /dev/null' - copr-fe`
		`- runuser -c '/usr/share/copr/coprs_frontend/run/check_for_anitya_version_updates.py --backend rubygems --delta=172800 &> /dev/null' - copr-fe`
		`runuser -c '/usr/share/copr/coprs_frontend/manage.py update_indexes_quick 120 &> /dev/null' - copr-fe`
		`runuser -c '/usr/share/copr/coprs_frontend/manage.py update_graphs &> /dev/null' - copr-fe`

frontend/conf/cron.hourly/copr-frontend-optional

file added

		`@@ -0,0 +1,7 @@`
		`+ #! /bin/sh`
		`+`
		`+ # Optional Copr frontend tasks to be executed hourly.`
		`+`
		`+ #runuser -c '/usr/share/copr/coprs_frontend/run/check_for_anitya_version_updates.py --backend pypi --delta=172800 &> /dev/null' - copr-fe`
		`+ #runuser -c '/usr/share/copr/coprs_frontend/run/check_for_anitya_version_updates.py --backend rubygems --delta=172800 &> /dev/null' - copr-fe`
		`+`

frontend/copr-frontend.spec

file modified

+5 -4

		`@@ -229,8 +229,8 @@`
		`install -d %{buildroot}/%{_bindir}`
		`install -d %{buildroot}%{_unitdir}`

		`- install -p -m 755 conf/cron.hourly/copr-frontend %{buildroot}%{_sysconfdir}/cron.hourly/copr-frontend`
		`- install -p -m 755 conf/cron.daily/copr-frontend %{buildroot}%{_sysconfdir}/cron.daily/copr-frontend`
		`+ install -p -m 755 conf/cron.hourly/copr-frontend* %{buildroot}%{_sysconfdir}/cron.hourly`
		`+ install -p -m 755 conf/cron.daily/copr-frontend* %{buildroot}%{_sysconfdir}/cron.daily`

		`cp -a coprs_frontend/* %{buildroot}%{_datadir}/copr/coprs_frontend`
		`sed -i "s/__RPM_BUILD_VERSION/%{version}-%{release}/" %{buildroot}%{_datadir}/copr/coprs_frontend/coprs/templates/layout.html`
		`@@ -327,8 +327,9 @@`
		`%config(noreplace) %{_sysconfdir}/copr/copr_devel.conf`
		`%config(noreplace) %{_sysconfdir}/copr/copr_unit_test.conf`

		`- %config(noreplace) %attr(0755, root, root) %{_sysconfdir}/cron.hourly/copr-frontend`
		`- %config(noreplace) %attr(0755, root, root) %{_sysconfdir}/cron.daily/copr-frontend`
		`+ %config %attr(0755, root, root) %{_sysconfdir}/cron.hourly/copr-frontend`
		`+ %config %attr(0755, root, root) %{_sysconfdir}/cron.daily/copr-frontend`
		`+ %config(noreplace) %attr(0755, root, root) %{_sysconfdir}/cron.hourly/copr-frontend-optional`
		`%exclude_files flavor`
		`%exclude_files devel`

no initial comment

rebased onto d1792dec1174f048130372c8c2b64a5f0c05bf53

5 years ago

frostyx commented 5 years ago

Only tasks which make no sense to disable should live here

I don't understand this sentence

If I would have to guess, I would say, that cron files get replaced by roles/copr/frontend-cloud/files/cron.*/copr-frontend from fedora ansible repo. I am not sure, though.

I think, that not having the cron file in ansible, and installing it from copr-frontend package will simplify things for our Copr instance, but complicate things for maintainers of other possible instances. There is IMHO no problem with having not-noreplace cron files with basic stuff like cleaning graph data, but I don't think, that we want to e.g. enable anitya rebuilds from not-noreplace cron file. That will be annoying even for internal Copr.

What is the motivation for this PR, maybe I am missing something.

praiskup commented 5 years ago

On Saturday, April 20, 2019 1:24:32 PM CEST Jakub Kadl=C4=8D=C3=ADk wrote:

frostyx commented on the pull-request: [frontend] replace cron configuration automatically that you are following: ``
=20

Only tasks which make no sense to disable should live here=20

I don't understand this sentence

It is valid point. I should use better wording.

If I would have to guess, I would say, that cron files get replaced by
roles/copr/frontend-cloud/files/cron.*/copr-frontend from fedora
ansible repo. I am not sure, though.

I think, that not having the cron file in ansible, and installing it from
copr-frontend package will simplify things for our Copr instance, but
complicate things for maintainers of other possible instances.

I'd like to have clear cut between "optional" stuff and "mandatory" stuff.

You must feel it when you call it "complicate things for maintainers". If
it is like that, we shouldn't even install it as %noreplace.

And for the basic stuff (say "delete_old_builds", etc.) it is a pity to
require every deployment to adjust deployment scripting...

There is IMHO no problem with having not-noreplace cron files with basic
stuff like cleaning graph data, but I don't think, that we want to e.g.
enable anitya rebuilds from not-noreplace cron file. That will be
annoying even for internal Copr.

Agreed, anitya should be clearly opt-in. I noticed this as well ... and
I think we should either remove it entirely (preferred) or at least have
two cron config files (one %noreplace, and one without that).

What is the motivation for this PR, maybe I am missing something.

I noticed we have new version of copr-frontend on staging, but the two new
cron jobs added were not on place. This sounds like gap to me.

To reply, visit the link below or just reply to this email
https://pagure.io/copr/copr/pull-request/675

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

5 years ago

praiskup commented 5 years ago

You must feel it when you call it "complicate things for maintainers". If
it is like that, we shouldn't even install it as %noreplace.

I mean, even with %noreplace you get in installed initially... So the %noreplace doesn't help anyone (not to Fedora instance, since it still needs the ansible script, not to internal copr -- it needs ansible script to disable it, and not to us as maintainers who want to change the defaults).

rebased onto da81c698733892e70aa39a22e2e03eeb87df73b3

5 years ago

rebased onto 12accb88939f406ba0c12192ca455f01c7904f0c

5 years ago

rebased onto 19f2d47

5 years ago

Metadata Update from @praiskup:
- Pull-request untagged with: meeting

5 years ago

Pull-Request has been merged by praiskup

5 years ago