Bug 32093 - cleanup_database.pl bg-jobs parameter should be bg-days
Summary: cleanup_database.pl bg-jobs parameter should be bg-days
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords: rel_22_11_candidate
Depends on: 31969
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-03 14:24 UTC by Nick Clemens (kidclamp)
Modified: 2023-06-08 22:28 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Bug 32093: Make help and code match for bg-days in cleanup_database.pl (2.48 KB, patch)
2022-11-15 16:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32093: Make help and code match for bg-days in cleanup_database.pl (2.53 KB, patch)
2022-11-15 20:17 UTC, David Nind
Details | Diff | Splinter Review
Bug 32093: Make help and code match for bg-days in cleanup_database.pl (2.58 KB, patch)
2022-11-17 17:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2022-11-03 14:24:17 UTC
The help says:
--bg-days DAYS          Purge all finished background jobs this many days old. Defaults to 1 if no DAYS provide    d.


The script checks:
'bg-jobs:i'         => \$background_days,

We should use bg-days
Comment 1 Katrin Fischer 2022-11-15 16:57:19 UTC
Created attachment 143921 [details] [review]
Bug 32093: Make help and code match for bg-days in cleanup_database.pl

There as a mismatch between the parameter name in documentation (bg-days)
and what was checked in the code (bg-jobs). This makes both match.

To test:
* Apply patch
* Make sure you have some background_jobs older than one day
* Verify the help of the script documents the parameter bg-days
* Run the cleanup_database.pl job
  Example:  ./misc/cronjobs/cleanup_database.pl --bg-days  --confirm -v
* Verify there are no errors and the lines have been deleted as expected
Comment 2 Katrin Fischer 2022-11-15 16:59:29 UTC
As this new option has not been released yet, we should push this before release so people can have it right from the beginning.
Comment 3 David Nind 2022-11-15 20:17:43 UTC
Created attachment 143930 [details] [review]
Bug 32093: Make help and code match for bg-days in cleanup_database.pl

There as a mismatch between the parameter name in documentation (bg-days)
and what was checked in the code (bg-jobs). This makes both match.

To test:
* Apply patch
* Make sure you have some background_jobs older than one day
* Verify the help of the script documents the parameter bg-days
* Run the cleanup_database.pl job
  Example:  ./misc/cronjobs/cleanup_database.pl --bg-days  --confirm -v
* Verify there are no errors and the lines have been deleted as expected

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2022-11-15 20:24:44 UTC
Testing notes (using KTD):

1. To create some background jobs that could be purged (I'm sure there are quicker ways), I:
   - Enabled Elasticsearch (SearchEngine)
   - Reindexed (koha-elasticsearch --rebuild -d -b -a kohadev)
   - Edited some records (changed titles)
   - Updated the dates in the database for background jobs so that they are more than 1 day old (wasn't sure what date is used to trigger pruging):
     . select * from background_jobs;
     . update background_jobs set enqueued_on = "2022-11-13 20:09:00";
     . update background_jobs set started_on = "2022-11-13 20:09:00";
     . update background_jobs set ended_on = "2022-11-13 20:10:00";
   - Edited a few more records (changed title)

2. Ran the cleanup_database.pl job.

3. Checked that the older background jobs were deleted in the database and UI.
Comment 5 Tomás Cohen Arazi 2022-11-17 17:56:06 UTC
Created attachment 144020 [details] [review]
Bug 32093: Make help and code match for bg-days in cleanup_database.pl

There as a mismatch between the parameter name in documentation (bg-days)
and what was checked in the code (bg-jobs). This makes both match.

To test:
* Apply patch
* Make sure you have some background_jobs older than one day
* Verify the help of the script documents the parameter bg-days
* Run the cleanup_database.pl job
  Example:  ./misc/cronjobs/cleanup_database.pl --bg-days  --confirm -v
* Verify there are no errors and the lines have been deleted as expected

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2022-11-17 18:04:33 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!