Merge lp:~paulgear/python-oops-datedir-repo/variable-retention-period into lp:python-oops-datedir-repo

Proposed by Paul Gear
Status: Needs review
Proposed branch: lp:~paulgear/python-oops-datedir-repo/variable-retention-period
Merge into: lp:python-oops-datedir-repo
Diff against target: 46 lines (+8/-7)
1 file modified
oops_datedir_repo/prune.py (+8/-7)
To merge this branch: bzr merge lp:~paulgear/python-oops-datedir-repo/variable-retention-period
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Benji York Pending
William Grant Pending
Review via email: mp+289863@code.launchpad.net

Description of the change

Landscape have requested longer retention for OOPSes; this is a presently-untested change to allow that. cRT#89955

To post a comment you must log in.
59. By Paul Gear

Update for log directory used in production

60. By Paul Gear

Specify option type

Revision history for this message
Данило Шеган (danilo) wrote :

Looks good, other than the changed default value for filename. If there's no particular reason to change that, it seems best to leave it as-is.

review: Approve
Revision history for this message
Paul Gear (paulgear) wrote :

> Looks good, other than the changed default value for filename. If there's no
> particular reason to change that, it seems best to leave it as-is.

My commit message seems to indicate that that's what is used by the running copy of this code in Launchpad production. I'd have to go back and confirm, but that's the only reason I can think of that I would have changed it.

If you aren't comfortable doing that, feel free to edit out when merging and we can maintain the patch locally in production.

Revision history for this message
Данило Шеган (danilo) wrote :

@Paul: do you keep a patch or pass the option instead?

Are you aware of any other users of oops code that might be affected (we've got Launchpad and Landscape).

Anyway, I am fine with it, but it generally belongs in a separate branch, though it is very small.

Revision history for this message
Paul Gear (paulgear) wrote :

@danilo: This was requested for Landscape by @benji. I'm not aware of any other users, but this is my first exposure to this codebase, so I'm not authoritative on it.

Revision history for this message
Данило Шеган (danilo) wrote :

@Paul: sure, I am on the Landscape team, so as a former Launchpad developer (with review rights), I've decided to look at this to clean up the queue (or, tbh, I've been prodded by Adam Collard to do it :)).

I was wondering if IS knows where else is this code deployed so we can contain the risk of any change? Eg. I suspect at least Launchpad is using it, and I don't want it to fall over because there is no "logs" subdirectory somewhere.

As for merging, it is usually much easier and safer if you revert the change in question, so when "landing", I can simply merge your changes.

Revision history for this message
Paul Gear (paulgear) wrote :

@danilo: I suspect Launchpad is using this, but I couldn't say where other than by using a brute force method.
@wgrant: Any suggestions on where to start looking?

Revision history for this message
William Grant (wgrant) wrote :

The only place LP uses this nowadays is with the rest on neem. The hardcoded log directory is already unpleasant, but I think the change here makes it even a bit worse. I'd revert the logging change for now, and leave it as a local change on neem.

Revision history for this message
Данило Шеган (danilo) wrote :

@Paul: I guess we all kind of agree to drop the change to the logging filename and then we can get it merged.

@William: does this code use PQM to land stuff (yes, it's been a while :)), or should we land it in some other way (just approve for tarmac, manually...)?

Revision history for this message
William Grant (wgrant) wrote :

On 14/10/16 17:46, Данило Шеган wrote:
> @Paul: I guess we all kind of agree to drop the change to the logging filename and then we can get it merged.
>
> @William: does this code use PQM to land stuff (yes, it's been a while :)), or should we land it in some other way (just approve for tarmac, manually...)?

It's owned by ~canonical-launchpad-branches, of which you are a member.
Just land it manually.

Revision history for this message
Данило Шеган (danilo) wrote :

У пет, 14. 10 2016. у 07:27 +0000, William Grant пише:
>
> It's owned by ~canonical-launchpad-branches, of which you are a
> member.
> Just land it manually.

Great, will do that as soon as the logging change is reverted. Thanks!

61. By Paul Gear

Revert default log location change

Revision history for this message
Paul Gear (paulgear) wrote :

Apologies for the delay on this; I've reverted that log location change as requested.

Unmerged revisions

61. By Paul Gear

Revert default log location change

60. By Paul Gear

Specify option type

59. By Paul Gear

Update for log directory used in production

58. By Paul Gear

Remove unused code

57. By Paul Gear

Update documentation for --retention

56. By Paul Gear

Allow use of different retention periods

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'oops_datedir_repo/prune.py'
--- oops_datedir_repo/prune.py 2012-09-26 06:57:23 +0000
+++ oops_datedir_repo/prune.py 2017-01-30 04:35:30 +0000
@@ -28,7 +28,6 @@
28import sys28import sys
2929
30from launchpadlib.launchpad import Launchpad30from launchpadlib.launchpad import Launchpad
31from launchpadlib.uris import lookup_service_root
32from pytz import utc31from pytz import utc
3332
34import oops_datedir_repo33import oops_datedir_repo
@@ -106,8 +105,8 @@
106 --project and --projectgroup can be supplied multiple times.105 --project and --projectgroup can be supplied multiple times.
107106
108 When run this program will ask Launchpad for OOPS references made since107 When run this program will ask Launchpad for OOPS references made since
109 the last date it pruned up to, with an upper limit of one week from108 the last date it pruned up to, using the upper limit specified with
110 today. It then looks in the repository for all oopses created during109 --retention. It then looks in the repository for all oopses created during
111 that date range, and if they are not in the set returned by Launchpad,110 that date range, and if they are not in the set returned by Launchpad,
112 deletes them. If the repository has never been pruned before, it will111 deletes them. If the repository has never been pruned before, it will
113 pick the earliest datedir present in the repository as the start date.112 pick the earliest datedir present in the repository as the start date.
@@ -122,6 +121,9 @@
122 help="Launchpad project group to find references in.")121 help="Launchpad project group to find references in.")
123 parser.add_option('--repo', help="Path to the repository to read from.")122 parser.add_option('--repo', help="Path to the repository to read from.")
124 parser.add_option(123 parser.add_option(
124 '--retention', help="Number of days to keep OOPSes", default=7,
125 type="int")
126 parser.add_option(
125 '--lpinstance', help="Launchpad instance to use", default="production")127 '--lpinstance', help="Launchpad instance to use", default="production")
126 options, args = parser.parse_args(argv[1:])128 options, args = parser.parse_args(argv[1:])
127 def needed(*optnames):129 def needed(*optnames):
@@ -140,10 +142,9 @@
140 logging.basicConfig(142 logging.basicConfig(
141 filename='prune.log', filemode='w', level=logging.DEBUG)143 filename='prune.log', filemode='w', level=logging.DEBUG)
142 repo = oops_datedir_repo.DateDirRepo(options.repo)144 repo = oops_datedir_repo.DateDirRepo(options.repo)
143 one_week = datetime.timedelta(weeks=1)145 retention = datetime.timedelta(days=options.retention)
144 one_day = datetime.timedelta(days=1)146 # Only prune OOPS reports older than the retention period
145 # Only prune OOPS reports more than one week old.147 prune_until = datetime.datetime.now(utc) - retention
146 prune_until = datetime.datetime.now(utc) - one_week
147 # Ignore OOPS reports we already found references for - older than the last148 # Ignore OOPS reports we already found references for - older than the last
148 # prune date.149 # prune date.
149 try:150 try:

Subscribers

People subscribed via source and target branches

to all changes: