#6 Add a command to operate on stale review tickets
Merged 3 years ago by cverna. Opened 3 years ago by mattia.

file modified
+1 -1
@@ -5,4 +5,4 @@ 

      dnf clean all &&\

      pip install git+https://pagure.io/Fedora-Infra/review_stats.git

  

- ENTRYPOINT ["review-stats", "-d", "/review-stats/", "-c", "/etc/review-stats/config.cfg"]

+ ENTRYPOINT ["review-stats", "-c", "/etc/review-stats/config.cfg", "make-pages", "-d", "/review-stats/"]

file modified
+16 -9
@@ -5,12 +5,14 @@ 

  Description

  -----------

  This tool is used by fedora-infrastructure to generate some HTML pages with useful statistics about new package review tickets opened in Red Hat Bugzilla.

+ It's also responsible for managing stale review tickets, asking for submitter or reviewer updates and

+ closing or resetting the ticket state when appropriate.

  

  Usage

  -----

  .. code:: bash

  

-    review-stats -d <OUTPUT_HTML_DIR>

+    review-stats make-pages -d <OUTPUT_HTML_DIR>

  

  `review_stats` requires a configuration file to be set up before usage. An example `review-stats.cfg` is provided by the package.

  
@@ -36,19 +38,24 @@ 

  

  **mail_to**: comma separated list of email addresses.

  

- CLI parameters

- ^^^^^^^^^^^^^^

+ Base command parameters

+ ^^^^^^^^^^^^^^^^^^^^^^^

  **-c, --config FILENAME**: the config file to use. By default the script will automatically search for it in `/etc/`.

  

- **-t, --templatedir PATH**: a path to the directory where HTML templates are stored. By default the script will automatically search for them in the package install dir.

- 

- ***-s, --staticfilesdir PATH**: a path to the directory where static files are stored. By default the script will automatically search for them in the package install dir.

- 

- **-d, --destination PATH [required]**: the path where generated HTML files should be copied.

- 

  **-l, --enable-systemd-log**: enable logging to system's journal via systemd.

  

  **-v, --verbose**: set console output to INFO level.

  

  **-D, --debug**: set console output to DEBUG level.

  

+ make-pages parameters

+ ^^^^^^^^^^^^^^^^^^^^^

+ **-t, --templatedir PATH**: a path to the directory where HTML templates are stored. By default the script will automatically search for them in the package install dir.

+ 

+ ***-s, --staticfilesdir PATH**: a path to the directory where static files are stored. By default the script will automatically search for them in the package install dir.

+ 

+ **-d, --destination PATH [required]**: the path where generated HTML files should be copied.

+ 

+ work-on-bugs parameters

+ ^^^^^^^^^^^^^^^^^^^^^^^

+ **-d, --dry-run**: do not really operate on bugs, but only output debug messages.

file modified
+47 -1
@@ -27,7 +27,7 @@ 

  

  import logging as python_logging

  

- __version__ = '5.1.0'

+ __version__ = '6.0.0'

  

  # Some magic bug numbers

  ACCEPT = 163779
@@ -43,4 +43,50 @@ 

  # These will show up in a query but aren't actual review tickets

  trackers = set([ACCEPT, BUNDLED, FEATURE, NEEDSPONSOR, GUIDELINES, SCITECH, SECLAB])

  

+ # Comments

+ comments = {

+     'close_needinfo': '''This is an automatic action taken by review-stats script.

+ 

+ The ticket submitter failed to clear the NEEDINFO flag in a month.

+ As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

+ we consider this ticket as DEADREVIEW and proceed to close it.

+ ''',

+     'reset_needinfo': '''This is an automatic action taken by review-stats script.

+ 

+ The ticket reviewer failed to clear the NEEDINFO flag in a month.

+ As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

+ we reset the status and the assignee of this ticket.

+ ''',

+     'needinfo_reviewer': '''This is an automatic check from review-stats script.

+ 

+ This review request ticket hasn't been updated for some time, but it seems

+ that the review is still being working out by you. If this is right, please

+ respond to this comment clearing the NEEDINFO flag and try to reach out the

+ submitter to proceed with the review.

+ 

+ If you're not interested in reviewing this ticket anymore, please clear the

+ fedora-review flag and reset the assignee, so that a new reviewer can take

+ this ticket.

+ 

+ Without any reply, this request will shortly be resetted.

+ ''',

+     'needinfo_submitter': '''This is an automatic check from review-stats script.

+ 

+ This review request ticket hasn't been updated for some time. We're sorry

+ it is taking so long. If you're still interested in packaging this software

+ into Fedora repositories, please respond to this comment clearing the

+ NEEDINFO flag.

+ 

+ You may want to update the specfile and the src.rpm to the latest version

+ available and to propose a review swap on Fedora devel mailing list to increase

+ chances to have your package reviewed. If this is your first package and you

+ need a sponsor, you may want to post some informal reviews. Read more at

+ https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

+ 

+ Without any reply, this request will shortly be considered abandoned

+ and will be closed.

+ Thank you for your patience.

+ ''',

+ }

+ 

  log = python_logging.getLogger('review_stats.py')

@@ -4,6 +4,10 @@ 

  password = <BUGZILLA_PASSWORD>

  # bugzilla_api_key = KEY

  

+ [review-stats-worker]

+ needinfo_waiting_days = 30

+ not_updated_days = 365

+ 

  [review-stats-logging]

  journal_level = INFO

  # Be aware these adresses will receive one email for each log message sent to the queue

@@ -1,1 +1,2 @@ 

- 0,30 * * * * apache /usr/local/bin/review-stats.py -d /srv/web/review-stats/

+ 0,30 * * * * apache /usr/local/bin/review-stats.py make-pages -d /srv/web/review-stats/

+ 0 15 * * * apache /usr/local/bin/review-stats.py work-on-bugs

@@ -25,7 +25,7 @@ 

  

  """A script for the new package review tickets page generation."""

  

- from datetime import datetime

+ from datetime import datetime, timedelta

  from jinja2 import Environment, FileSystemLoader

  from pkg_resources import resource_filename

  import click
@@ -33,37 +33,34 @@ 

  import sys

  import time

  

- from review_stats import log, __version__ as version

+ from review_stats import comments, log, __version__ as version

  from review_stats.logging import setup_logging

- from review_stats.utils import (

-     bug_time, bz_connect, copy_static_content, generate_page, get_bz_baseurl,

-     get_hidden, get_in_progress, get_reviewable, get_reviewable_epel, get_reviewers,

-     get_top_blockers, get_top_submitters, order_by_month, run_query)

+ from review_stats import utils

  

  

- @click.command()

+ @click.group()

  @click.option('-c', '--config', 'configfile', type=click.File(),

                default='/etc/review-stats.cfg', show_default=True)

- @click.option('-t', '--templatedir', 'templdir', type=click.Path(exists=True),

-               default=resource_filename('review_stats', 'templates/'), show_default=True)

- @click.option('-s', '--staticfilesdir', 'staticdir', type=click.Path(exists=True),

-               default=resource_filename('review_stats', 'static/'), show_default=True)

- @click.option('-d', '--destination', 'dirname', type=click.Path(exists=True, writable=True),

-               required=True)

  @click.option('-l', '--enable-systemd-log', 'systemd_log', is_flag=True)

  @click.option('-v', '--verbose', 'loglevel', flag_value='INFO')

  @click.option('-D', '--debug', 'loglevel', flag_value='DEBUG')

- def make_page(configfile, templdir, staticdir, dirname, systemd_log, loglevel):

-     """Builds Fedora's new package review tickets page."""

+ @click.pass_context

+ def cli(ctx, configfile, systemd_log, loglevel):

+     """Setup common environment for commands."""

+     ctx.ensure_object(dict)

+ 

      loglevel = loglevel or 'WARNING'

+     ctx.obj['loglevel'] = loglevel

  

      config = configparser.ConfigParser()

      config.read_file(configfile)

+     ctx.obj['config'] = config

  

      setup_logging(loglevel, config, systemd_log)

  

      try:

-         bz = bz_connect(config)

+         bz = utils.bz_connect(config)

+         ctx.obj['bz'] = bz

      except configparser.NoOptionError as ex:

          log.critical(f'Missing required value in configuration file: {ex.section}:{ex.option}')

          sys.exit(1)
@@ -72,111 +69,163 @@ 

          sys.exit(1)

  

      try:

-         bz_tickets = run_query(bz)

+         bz_tickets = utils.run_query(bz)

+         ctx.obj['bz_tickets'] = bz_tickets

      except Exception:

          log.critical('Something went wrong while fetching data from Bugzilla. '

                       'I can\'t proceed further.', exc_info=1)

          sys.exit(1)

  

+ 

+ @cli.command()

+ @click.option('-t', '--templatedir', 'templdir', type=click.Path(exists=True),

+               default=resource_filename('review_stats', 'templates/'), show_default=True)

+ @click.option('-s', '--staticfilesdir', 'staticdir', type=click.Path(exists=True),

+               default=resource_filename('review_stats', 'static/'), show_default=True)

+ @click.option('-d', '--destination', 'dirname', type=click.Path(exists=True, writable=True),

+               required=True)

+ @click.pass_context

+ def make_pages(ctx, templdir, staticdir, dirname):

+     """Build Fedora's new package review tickets pages."""

      env = Environment(loader=FileSystemLoader(templdir), autoescape=True)

      env.globals['now'] = datetime.utcnow

      env.globals['version'] = version

-     env.globals['bz_baseurl'] = get_bz_baseurl(config)

-     env.filters['bugtime'] = bug_time

+     env.globals['bz_baseurl'] = utils.get_bz_baseurl(ctx.obj['config'])

+     env.filters['bugtime'] = utils.bug_time

  

      log.info('Starting HTML page rendering...')

      counters = {}

      t = time.time()

  

+     bz_tickets = ctx.obj['bz_tickets']

+ 

      # Reviewable Fedora tickets

-     new_tickets = get_reviewable(bz_tickets)

+     new_tickets = utils.get_reviewable(ctx.obj['bz_tickets'])

      counters['reviewable'] = len(new_tickets)

-     generate_page(env, 'by_month.html', dirname, 'reviewable.html',

+     utils.generate_page(env, 'by_month.html', dirname, 'reviewable.html',

                    page_title='New, reviewable Fedora package review tickets',

-                   months=order_by_month(new_tickets),

+                   months=utils.order_by_month(new_tickets),

                    count=counters['reviewable'])

  

      # Reviewable EPEL tickets

-     new_epel_tickets = get_reviewable_epel(bz_tickets)

+     new_epel_tickets = utils.get_reviewable_epel(ctx.obj['bz_tickets'])

      counters['reviewable_epel'] = len(new_epel_tickets)

-     generate_page(env, 'by_month.html', dirname, 'reviewable_epel.html',

+     utils.generate_page(env, 'by_month.html', dirname, 'reviewable_epel.html',

                    page_title='New, reviewable EPEL only package review tickets',

-                   months=order_by_month(new_epel_tickets),

+                   months=utils.order_by_month(new_epel_tickets),

                    count=counters['reviewable_epel'])

  

      # Reviews in progress

-     in_progress_tickets = get_in_progress(bz_tickets)

+     in_progress_tickets = utils.get_in_progress(ctx.obj['bz_tickets'])

      counters['in_progress'] = len(in_progress_tickets)

-     generate_page(env, 'in_progress.html', dirname, 'in_progress.html',

+     utils.generate_page(env, 'in_progress.html', dirname, 'in_progress.html',

                    page_title='All tickets currently under review',

                    tickets=sorted(in_progress_tickets, key=lambda t: t.last_modified),

                    count=counters['in_progress'])

  

      # Hidden tickets

-     hidden_tickets = get_hidden(bz_tickets)

+     hidden_tickets = utils.get_hidden(ctx.obj['bz_tickets'])

      counters['hidden'] = len(hidden_tickets)

-     generate_page(env, 'hidden.html', dirname, 'hidden.html',

+     utils.generate_page(env, 'hidden.html', dirname, 'hidden.html',

                    page_title='All tickets hidden from the main review queues',

-                   months=order_by_month(hidden_tickets),

+                   months=utils.order_by_month(hidden_tickets),

                    count=counters['hidden'])

  

      # Inconsistent tickets

-     inconsistent_tickets = [tk for tk in bz_tickets if tk.has_inconsistent_state]

+     inconsistent_tickets = [tk for tk in ctx.obj['bz_tickets'] if tk.has_inconsistent_state]

      counters['inconsistent'] = len(inconsistent_tickets)

-     generate_page(env, 'plain.html', dirname, 'inconsistent.html',

+     utils.generate_page(env, 'plain.html', dirname, 'inconsistent.html',

                    page_title='All review tickets which are in limbo',

                    tickets=sorted(inconsistent_tickets, key=lambda t: t.last_modified),

                    count=counters['inconsistent'])

  

      # Needsponsor tickets

      # Some of them are in the in progress queue, but still waiting for a sponsor

-     needsponsor_tickets = [tk for tk in bz_tickets if not tk.hidden and tk.needsponsor]

+     needsponsor_tickets = [tk for tk in ctx.obj['bz_tickets'] if not tk.hidden and tk.needsponsor]

      counters['needsponsor'] = len(needsponsor_tickets)

-     generate_page(env, 'plain.html', dirname, 'needsponsor.html',

+     utils.generate_page(env, 'plain.html', dirname, 'needsponsor.html',

                    page_title='All review tickets where a sponsor is required',

                    tickets=sorted(needsponsor_tickets, key=lambda t: t.last_modified),

                    count=counters['needsponsor'])

  

      # Trivial tickets

-     trivial_tickets = [tk for tk in get_reviewable(bz_tickets) + get_reviewable_epel(bz_tickets)

+     trivial_tickets = [tk for tk in utils.get_reviewable(ctx.obj['bz_tickets'])

+                        + utils.get_reviewable_epel(ctx.obj['bz_tickets'])

                         if not tk.hidden and tk.trivial]

      counters['trivial'] = len(trivial_tickets)

-     generate_page(env, 'by_month.html', dirname, 'trivial.html',

+     utils.generate_page(env, 'by_month.html', dirname, 'trivial.html',

                    page_title='All tickets marked as trivial',

-                   months=order_by_month(trivial_tickets),

+                   months=utils.order_by_month(trivial_tickets),

                    count=counters['trivial'])

  

      # Top blockers

-     top_blockers = get_top_blockers(bz_tickets)

+     top_blockers = utils.get_top_blockers(ctx.obj['bz_tickets'])

      counters['blockers'] = len(top_blockers)

-     generate_page(env, 'blockers.html', dirname, 'blockers.html',

+     utils.generate_page(env, 'blockers.html', dirname, 'blockers.html',

                    page_title='All tickets which block other tickets',

                    blockers=top_blockers,

                    count=counters['blockers'])

  

      # Top submitters

-     top_submitters = get_top_submitters(bz_tickets)

+     top_submitters = utils.get_top_submitters(ctx.obj['bz_tickets'])

      counters['submitters'] = len(top_submitters)

-     generate_page(env, 'by_user.html', dirname, 'submitters.html',

+     utils.generate_page(env, 'by_user.html', dirname, 'submitters.html',

                    page_title='All review tickets sorted by submitter',

                    users=top_submitters,

                    count=counters['submitters'])

  

      # Reviewers

-     reviewers = get_reviewers(bz_tickets)

+     reviewers = utils.get_reviewers(ctx.obj['bz_tickets'])

      counters['reviewers'] = len(reviewers)

-     generate_page(env, 'by_user.html', dirname, 'reviewers.html',

+     utils.generate_page(env, 'by_user.html', dirname, 'reviewers.html',

                    page_title='Review tickets sorted by assigned reviewer',

                    users=reviewers,

                    count=counters['reviewers'])

  

      # Finally, generate the index page

-     generate_page(env, 'index.html', dirname, 'index.html',

+     utils.generate_page(env, 'index.html', dirname, 'index.html',

                    counters=counters)

      log.info(f'Done HTML rendering, took {time.time() - t:.2f}s.')

  

-     copy_static_content(staticdir, dirname)

+     utils.copy_static_content(staticdir, dirname)

+ 

+ 

+ @cli.command()

+ @click.option('-d', '--dry-run', 'dry', is_flag=True)

+ @click.pass_context

+ def work_on_bugs(ctx, dry):

+     """Operate on Bugzilla tickets."""

+     # Request info for tickets which were not recently updated

+     worker_config = ctx.obj['config']['review-stats-worker']

+     not_updated_days = timedelta(days=int(worker_config['not_updated_days']))

+     for tk in utils.get_not_recently_updated(ctx.obj['bz_tickets'], not_updated_days):

+         if not tk.review_flag_status or (tk.review_flag_status == '?' and not tk.really_assigned):

+             # No one is reviewing or review in progress but no assignee set

+             log.debug(f'Setting NEEDINFO from submitter for {tk.bug.id}.')

+             if not dry:

+                 utils.set_needinfo(ctx.obj['bz'], tk, tk.bug.creator,

+                                    comments['needinfo_submitter'])

+         elif tk.review_flag_status == '?' and tk.really_assigned:

+             # Review in progress and we're sure to have an assignee

+             log.debug(f'Setting NEEDINFO from reviewer for {tk.bug.id}.')

+             if not dry:

+                 utils.set_needinfo(ctx.obj['bz'], tk, tk.bug.assigned_to,

+                                    comments['needinfo_reviewer'])

+ 

+     not_approved_tickets = [tk for tk in ctx.obj['bz_tickets'] if tk.review_flag_status != '+']

+     needinfo_waiting_days = timedelta(days=int(worker_config['needinfo_waiting_days']))

+     # Close tickets with unanswered NEEDINFO from submitter

+     for tk in utils.get_needinfo_since(not_approved_tickets, needinfo_waiting_days, 'submitter'):

+         log.debug(f'Closing ticket {tk.bug.id} as dead review.')

+         if not dry:

+             utils.close_needinfo(ctx.obj['bz'], tk)

+ 

+     # Reset tickets with unanswered NEEDINFO from reviewer

+     for tk in utils.get_needinfo_since(not_approved_tickets, needinfo_waiting_days, 'reviewer'):

+         log.debug(f'Resetting ticket {tk.bug.id}.')

+         if not dry:

+             utils.reset_needinfo(ctx.obj['bz'], tk)

  

  

  if __name__ == '__main__':

-     make_page()

+     cli()

file modified
+110 -2
@@ -25,7 +25,8 @@ 

  

  """Utils for review_stats.py script."""

  

- from datetime import datetime

+ from datetime import datetime, timedelta

+ from typing import List

  from urllib.parse import urlparse

  import bugzilla

  import os
@@ -33,7 +34,7 @@ 

  import tempfile

  import time

  

- from . import log, trackers

+ from . import DEADREVIEW, comments, log, trackers

  from .models import ReviewTicket

  

  
@@ -93,6 +94,31 @@ 

      return bugtime.strftime(format)

  

  

+ def close_needinfo(bz: bugzilla.Bugzilla, tk: ReviewTicket):

+     """Close tickets as NOTABUG and mark it blocking FE-DEADREVIEW."""

+ 

+     # Clear needinfo and fedora-review flags

+     new_flags = [{'name': 'fedora-review', 'status': 'X'}]

+ 

+     # We may have more than one needinfo flag

+     # Let's clear only the one targeted to the ticket submitter

+     flags = tk.bug.flags

+     needinfo_flag_id = None

+     for idx, flag in enumerate(flags):

+         if 'name' in flag and flag['name'] == 'needinfo' and flag['requestee'] == tk.bug.creator:

+             needinfo_flag_id = flags[idx]['id']

+     if needinfo_flag_id:

+         new_flags.append({'id': needinfo_flag_id, 'status': 'X'})

+ 

+     update = bz.build_update(comment=comments['close_needinfo'], flags=new_flags,

+                              status='CLOSED', resolution='NOTABUG')

+     update['blocks'] = {'add': [DEADREVIEW]}

+     try:

+         bz.update_bugs([tk.bug.id], update)

+     except Exception:

+         log.error(f'Closing bug {tk.bug.id} failed: ', exc_info=1)

+ 

+ 

  def get_bz_baseurl(config):

      """Assume Bugzilla base URL from xmlrpc server URL in config."""

  
@@ -121,6 +147,38 @@ 

      return sorted(tklist, key=lambda b: b.bug.id)

  

  

+ def get_needinfo_since(tklist: List[ReviewTicket],

+                        delta: timedelta = timedelta(days=30),

+                        user: str = None) -> List[ReviewTicket]:

+     """Return a list of tickets waiting on needinfo from user for more than specified delta.

+ 

+     Args:

+         tklist: a list of review tickets

+         delta: a timedelta for considering a needinfo outdated. Defaults to 30 days.

+         user: 'submitter' or 'reviewer'

+     """

+ 

+     if user not in ['submitter', 'reviewer']:

+         raise ValueError('Invalid user type. Expected one of: submitter or reviewer')

+     date_limit = datetime.now() - delta

+     output = []

+     for tk in tklist:

+         user_email = tk.bug.creator if user == 'submitter' else tk.bug.assigned_to

+         if not tk.needinfo:

+             continue

+         for flag in tk.bug.flags:

+             if ('name' in flag and flag['name'] == 'needinfo'

+                     and flag['is_active'] == 1):

+                 date_set = flag['modification_date']

+                 req_user = flag['requestee']

+                 break

+         if date_set < date_limit:

+             if req_user == user_email:

+                 output.append(tk)

+ 

+     return sorted(output, key=lambda b: b.bug.id)

+ 

+ 

  def get_reviewable(tklist):

      """Return a list of reviewable tickets."""

  
@@ -141,6 +199,20 @@ 

      return sorted(tklist, key=lambda b: b.bug.id)

  

  

+ def get_not_recently_updated(tklist: List[ReviewTicket],

+                              delta: timedelta = timedelta(days=365)) -> List[ReviewTicket]:

+     """Return a list of tickets which were not recently updated.

+ 

+     Args:

+         tklist: a list of review tickets

+         delta: a timedelta for considering a ticket outdated. Defaults to 1 year.

+     """

+ 

+     date_limit = datetime.now() - delta

+     tklist = [tk for tk in tklist if tk.last_modified < date_limit]

+     return sorted(tklist, key=lambda b: b.bug.id)

+ 

+ 

  def get_top_blockers(tklist):

      """Return a dictionary with counters about open blockers."""

  
@@ -169,6 +241,42 @@ 

                                      key=lambda item: len(item[1]), reverse=True)}

  

  

+ def reset_needinfo(bz: bugzilla.Bugzilla, tk: ReviewTicket):

+     """Reset ticket status and assignee."""

+ 

+     # Clear needinfo and fedora-review flags

+     new_flags = [{'name': 'fedora-review', 'status': 'X'}]

+ 

+     # We may have more than one needinfo flag

+     # Let's clear only the one targeted to the ticket assignee

+     flags = tk.bug.flags

+     needinfo_flag_id = None

+     for idx, flag in enumerate(flags):

+         if 'name' in flag and flag['name'] == 'needinfo' and flag['requestee'] == tk.assignee:

+             needinfo_flag_id = flags[idx]['id']

+     if needinfo_flag_id:

+         new_flags.append({'id': needinfo_flag_id, 'status': 'X'})

+ 

+     update = bz.build_update(comment=comments['reset_needinfo'], flags=new_flags,

+                              status='NEW', reset_assigned_to=True)

+     try:

+         bz.update_bugs([tk.bug.id], update)

+     except Exception:

+         log.error(f'Resetting bug {tk.bug.id} failed: ', exc_info=1)

+ 

+ 

+ def set_needinfo(bz: bugzilla.Bugzilla, tk: ReviewTicket, email: str, comment: str):

+     """Set NEEDINFO status on ticket against specified user."""

+ 

+     flags = tk.bug.flags

+     flags.append({'name': 'needinfo', 'status': '?', 'requestee': email})

+     update = bz.build_update(comment=comment, flags=flags)

+     try:

+         bz.update_bugs([tk.bug.id], update)

+     except Exception:

+         log.error(f'Setting NEEDINFO on bug {tk.bug.id} failed: ', exc_info=1)

+ 

+ 

  def get_reviewers(tklist):

      """Return a dictionary with counters about reviewers."""

  

file modified
+1 -1
@@ -24,7 +24,7 @@ 

      ],

      entry_points='''

          [console_scripts]

-         review-stats=review_stats.scripts.review_stats:make_page

+         review-stats=review_stats.scripts.review_stats:cli

      ''',

      python_requires=">=3.6",

  )

As per discussion on https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/R2I6GPYRMSUNV44VBILGIJUB52E2ABMW/ this PR will make review_stats able to operate on stale review tickets.

The cli command responsible to generate webpages has been transferred to a subcommand (make-pages) with its options, while the common options stay with the main command.
A new subcommand work-on-bugs will operate on stale tickets. Currently it does:

  • Set NEEDINFO against submitter on tickets not assigned and not updated for more than one year
  • Set NEEDINFO against reviewer on tickets assigned and not updated for more than one year
  • Reset ticket status and assignee on tickets where the assignee has not replied to the NEEDINFO for more than 1 month
  • Close tickets where the submitter has not replied to the NEEDINFO for more than 1 month

Things to do along with (or after, since the html page generation should still work) this code change:

[ ] The PR already modify the docker entrypoint for the make-pages command to run. We will need to add another entrypoint for the work-on-bugs subcommand and set a daily cronjob for that.
[X] We also need the package-review user to be able to operate on bugs, since it currently hasn't got any privilege for that (I suppose I need to open a request on Infra).
[ ] It would be nice to see logs produced by the script, but in Openshift it seems the logs are deleted just after the run (?)

Signed-off-by: Mattia Verga mattia.verga@protonmail.com

Im going to wait a few days here for FESCo members to chime in if they have any concerns... ping on wed if we haven't merged it by then.

Thanks so much for working on all this!

rebased onto f305a0d

3 years ago

rebased onto 014c41b

3 years ago

I think this will be rather confusing for users, since they'll usually associate fedora-review with the commandline tool to do reviews. Maybe provide a link to the script sources or reference here?

It'd be better to use the native python syntax to have the text look like it will look for the users and avoid escaping:

comments = {
    'close_needinfo': '''\
This is an automatic action taken by Fedora-review script.
Escaping isn't needed.

The ticket submitter failed to clear the NEEDINFO flag in a month.
...
''',
    'reset_needinfo': '''\
...
}

In particular, this text has some lines which are very long, and it isn't even clear if this is
on purpose or because \n was forgotten.

I think this will be rather confusing for users, since they'll usually associate fedora-review with the commandline tool to do reviews. Maybe provide a link to the script sources or reference here?

Ah yes, you're right..... I constantly think review-stats and then I write fedora-review.... 🙄

rebased onto 281b948

3 years ago

rebased onto 56ea284

3 years ago

So, shall we merge and do this?

So, shall we merge and do this?

Well, since no objections has been made here or from FESCO about this automation (everyone seems happy with that).
So, I would merge this into master only, for the moment. When staging will be available again, I will try to deploy there and see if everything still works.

After that, I will need to make some other changes to the dockerfile here and to ansible config to actually make the "work-on-bugs" part working.

Pull-Request has been merged by cverna

3 years ago