| |
@@ -2,6 +2,7 @@
|
| |
|
| |
import datetime
|
| |
import re
|
| |
+ import collections
|
| |
|
| |
from blockerbugs import app
|
| |
from blockerbugs.util import pagure_interface
|
| |
@@ -24,6 +25,20 @@
|
| |
SUMMARY_HEADER = ""
|
| |
CLOSED_VOTES_HEADER = "The following votes have been closed:"
|
| |
|
| |
+ # Containers for parsed commands
|
| |
+ VoteCommand = collections.namedtuple('VoteCommand', ['tracker', 'vote'])
|
| |
+ VoteCommand.__doc__ += (
|
| |
+ ': A parsed vote command, e.g. tracker="betablocker", vote="+1"')
|
| |
+
|
| |
+ AgreedCommand = collections.namedtuple(
|
| |
+ 'AgreedCommand', ['tracker', 'outcome', 'summary'], defaults=[None])
|
| |
+ AgreedCommand.__doc__ += (
|
| |
+ ': A parsed AGREED command, e.g. tracker="betablocker", '
|
| |
+ 'outcome="accepted", summary="Breaks boot."')
|
| |
+
|
| |
+ RevoteCommand = collections.namedtuple('RevoteCommand', ['tracker'])
|
| |
+ RevoteCommand.__doc__ += ': A parsed REVOTE command, e.g. tracker="betablocker"'
|
| |
+
|
| |
|
| |
def agreed_revote_parser(line):
|
| |
"""Parse an AGREED or a REVOTE line. If there's anything violating the
|
| |
@@ -32,38 +47,46 @@
|
| |
and return []. All instances of "fe" get expanded to "freezeexception".
|
| |
|
| |
:param str line: a single line
|
| |
- :return: A list of commands with one tracker per line (or an empty list).
|
| |
- E.g. for line 'agreed acceptedbetablocker rejectedfinalfe' this returns:
|
| |
- ['agreed acceptedbetablocker',
|
| |
- 'agreed rejectedfinalfreezeexception']
|
| |
- For line 'revote finalblocker 0day' this returns:
|
| |
- ['revote finalblocker',
|
| |
- 'revote 0day']
|
| |
+ :return: A list of :class:`AgreedCommand` or :class:`RevoteCommand` instances.
|
| |
+ E.g. for the line 'agreed acceptedbetablocker rejectedfinalfe' this returns:
|
| |
+ [AgreedCommand(tracker='betablocker', outcome='accepted'),
|
| |
+ AgreedCommand(tracker='finalfreezeexception', outcome='rejected')]
|
| |
+ For the line 'revote finalblocker 0day' this returns:
|
| |
+ [RevoteCommand(tracker='finalblocker'),
|
| |
+ RevoteCommand(tracker='0day')]
|
| |
"""
|
| |
# parsing word by word avoids complex and fragile regex syntax
|
| |
words = line.split()
|
| |
- # first word must be 'agreed' or 'revote'
|
| |
if len(words) <= 0:
|
| |
return []
|
| |
- command = words[0]
|
| |
- if command == 'agreed':
|
| |
- matcher = OUTCOME_MATCHER
|
| |
- summary = True
|
| |
- elif command == 'revote':
|
| |
- matcher = TRACKER_MATCHER
|
| |
- summary = False
|
| |
|
| |
out = []
|
| |
+ command = words[0]
|
| |
for word in words[1:]:
|
| |
- if matcher.fullmatch(word):
|
| |
- # if we get a match, save it as a valid command
|
| |
- word = expand_fe(word)
|
| |
- out.append(f'{command} {word}')
|
| |
- else:
|
| |
- # if not, we break and return the matches we got so far if summary
|
| |
- # is allowed, return empty if not
|
| |
- if summary:
|
| |
+ if command == 'agreed':
|
| |
+ match = OUTCOME_MATCHER.fullmatch(word)
|
| |
+ if (match):
|
| |
+ ac = AgreedCommand(
|
| |
+ tracker=expand_fe(match.group(2)),
|
| |
+ outcome=match.group(1),
|
| |
+ )
|
| |
+ out.append(ac)
|
| |
+ else:
|
| |
+ # this must be the summary now
|
| |
+ # FIXME: implement summary parsing
|
| |
break
|
| |
+ elif command == 'revote':
|
| |
+ match = TRACKER_MATCHER.fullmatch(word)
|
| |
+ if (match):
|
| |
+ rc = RevoteCommand(
|
| |
+ tracker=expand_fe(word),
|
| |
+ )
|
| |
+ out.append(rc)
|
| |
+ else:
|
| |
+ # this is not a valid REVOTE line
|
| |
+ return []
|
| |
+ else:
|
| |
+ # not an AGREED or REVOTE line
|
| |
return []
|
| |
return out
|
| |
|
| |
@@ -74,26 +97,32 @@
|
| |
invalidates the line. All instances of "fe" get expanded to "freezeexception".
|
| |
|
| |
:param str line: a line which might contain a vote
|
| |
- :return: A list of votes with one tracker per line (or an empty list).
|
| |
- E.g. for line 'betafe +1 finalblocker -1' this returns:
|
| |
- ['betafreezeexception +1',
|
| |
- 'finalblocker -1']
|
| |
+ :return: A list of :class:`VoteCommand` instances.
|
| |
+ E.g. for the line 'betafe +1 finalblocker -1' this returns:
|
| |
+ [VoteCommand(tracker='betafreezeexception', vote='+1'),
|
| |
+ VoteCommand(tracker='finalblocker', vote='-1')]
|
| |
"""
|
| |
- out = []
|
| |
words = line.split()
|
| |
# if we have an odd number of words we don't have a clean vote line
|
| |
if (len(words) % 2) != 0:
|
| |
- return out
|
| |
+ return []
|
| |
+
|
| |
+ out = []
|
| |
# split list into pairs:
|
| |
# https://stackoverflow.com/questions/312443
|
| |
for pair in (words[i:i + 2] for i in range(0, len(words), 2)):
|
| |
if TRACKER_MATCHER.fullmatch(pair[0]) and pair[1] in VOTES:
|
| |
- pair[0] = expand_fe(pair[0])
|
| |
- out.append(" ".join(pair))
|
| |
+ vc = VoteCommand(
|
| |
+ tracker=expand_fe(pair[0]),
|
| |
+ vote=pair[1]
|
| |
+ )
|
| |
+ out.append(vc)
|
| |
elif pair[0] in VOTES and TRACKER_MATCHER.fullmatch(pair[1]):
|
| |
- pair[1] = expand_fe(pair[1])
|
| |
- pair.reverse()
|
| |
- out.append(" ".join(pair))
|
| |
+ vc = VoteCommand(
|
| |
+ tracker=expand_fe(pair[1]),
|
| |
+ vote=pair[0]
|
| |
+ )
|
| |
+ out.append(vc)
|
| |
else:
|
| |
# we found something other than a vote pair, so per
|
| |
# Kamil's Strict Vote Parsing Regime, we reject the line
|
| |
@@ -101,37 +130,14 @@
|
| |
return out
|
| |
|
| |
|
| |
- def expand_fe(keyword):
|
| |
- '''Expand "FE" into "FreezeException" in tracker-related keywords.
|
| |
-
|
| |
- While we accept "FE" as a handy user abbreviation, we want to process
|
| |
- it as "FreezeException" internally, so that we don't need to handle both
|
| |
- variants everywhere in our code.
|
| |
-
|
| |
- :param str keyword: a tracker or an outcome keyword, lowercased
|
| |
- :return: a keyword expanded to the full "FreezeException" variant if
|
| |
- applicable, or the keyword unchanged)
|
| |
- '''
|
| |
- tracker_match = TRACKER_MATCHER.fullmatch(keyword)
|
| |
- outcome_match = OUTCOME_MATCHER.fullmatch(keyword)
|
| |
-
|
| |
- if tracker_match:
|
| |
- # the 'fe' string is in group #3 in TRACKER_RE
|
| |
- fe_group_num = 3
|
| |
- match = tracker_match
|
| |
- elif outcome_match:
|
| |
- # the 'fe' string is in group #4 in OUTCOME_RE
|
| |
- fe_group_num = 4
|
| |
- match = outcome_match
|
| |
+ def expand_fe(tracker):
|
| |
+ '''Expand "fe" into "freezeexception" in tracker names.'''
|
| |
+ if tracker == 'betafe':
|
| |
+ return 'betafreezeexception'
|
| |
+ elif tracker == 'finalfe':
|
| |
+ return 'finalfreezeexception'
|
| |
else:
|
| |
- return keyword
|
| |
-
|
| |
- if match.group(fe_group_num) != 'fe':
|
| |
- return keyword
|
| |
-
|
| |
- start = match.start(fe_group_num)
|
| |
- end = start + len('fe')
|
| |
- return keyword[:start] + 'freezeexception' + keyword[end:]
|
| |
+ return tracker
|
| |
|
| |
|
| |
def bot_loop_detected(comments):
|
| |
@@ -168,15 +174,16 @@
|
| |
# FIXME it would be good to save/cache the data
|
| |
return self.user in pagure_interface.get_group('fedora-qa')['members']
|
| |
|
| |
- def keywords(self):
|
| |
- '''Search for command keywords in this comment, purge anything
|
| |
- unrelated and return it.
|
| |
+ def commands(self):
|
| |
+ '''Search for commands in this comment, purge anything unrelated and
|
| |
+ return it parsed.
|
| |
|
| |
- :return: a list with one vote or command per line, e.g.:
|
| |
- ['betablocker +1',
|
| |
- 'agreed acceptedbetablocker',
|
| |
- 'revote betablocker',
|
| |
- 'betablocker -1']
|
| |
+ :return: a list of :class:`VoteCommand`, :class:`AgreedCommand`, and
|
| |
+ :class:`RevoteCommand` instances, e.g.:
|
| |
+ [VoteCommand(tracker='betablocker', vote='+1'),
|
| |
+ AgreedCommand(tracker='betablocker', outcome='accepted'),
|
| |
+ RevoteCommand(tracker='betablocker'),
|
| |
+ VoteCommand(tracker='betablocker', vote='-1')]
|
| |
'''
|
| |
out = []
|
| |
for line in self.text.lower().split('\n'):
|
| |
@@ -232,22 +239,15 @@
|
| |
if comment.user == app.config['PAGURE_BOT_USERNAME']:
|
| |
return
|
| |
|
| |
- for words in [line.split() for line in comment.keywords()]:
|
| |
- agreed = (words[0] == 'agreed')
|
| |
- revote = (words[0] == 'revote')
|
| |
-
|
| |
- if agreed:
|
| |
- match = OUTCOME_MATCHER.fullmatch(words[1])
|
| |
- outcome = match.group(1)
|
| |
- tracker = match.group(2)
|
| |
- elif revote:
|
| |
- tracker = words[1]
|
| |
- else: # a vote
|
| |
- tracker = words[0]
|
| |
-
|
| |
- if tracker != self.tracker:
|
| |
+ for command in comment.commands():
|
| |
+ if command.tracker != self.tracker:
|
| |
+ # not intended for this tracker, ignore
|
| |
continue
|
| |
|
| |
+ agreed = isinstance(command, AgreedCommand)
|
| |
+ revote = isinstance(command, RevoteCommand)
|
| |
+ vote = isinstance(command, VoteCommand)
|
| |
+
|
| |
if (agreed or revote) and not comment.user_is_admin():
|
| |
app.logger.debug(
|
| |
f'A non-admin user {comment.user} tries to perform '
|
| |
@@ -257,21 +257,24 @@
|
| |
|
| |
if agreed:
|
| |
self.open = False
|
| |
- self.outcome = outcome
|
| |
+ self.outcome = command.outcome
|
| |
self.need_summary_post = True
|
| |
elif revote:
|
| |
self.open = True
|
| |
self.outcome = None
|
| |
self.need_summary_post = False
|
| |
self.votes = {}
|
| |
- else: # a vote
|
| |
+ elif vote:
|
| |
if not self.open:
|
| |
continue
|
| |
- assert words[1] in VOTES
|
| |
self.votes[comment.user] = {
|
| |
- 'vote': words[1],
|
| |
+ 'vote': command.vote,
|
| |
'comment_id': comment.id
|
| |
}
|
| |
+ else:
|
| |
+ msg = f'Unknown command instance: {command}'
|
| |
+ app.logger.error(msg)
|
| |
+ assert False, msg
|
| |
|
| |
def enumerate_votes(self):
|
| |
'''Return self.votes in a format:
|
| |
kparal
commented 3 years ago
Before, we filtered commands from a comment text, but still kept them in a text
form, and so when we wanted to process them in a later stage, we needed to parse
them again (or rely on unsafe modifications). This patch avoids that by using
data containers for parsed commands, and so the later stages can directly access
structured data instead of parsing the text again.
Related: https://pagure.io/fedora-qa/blockerbugs/pull-request/163
This is based on #163 branch code, so I created this PR against that. Of course this will go into develop, once #163 is merged.