From d17b24a4a4ad80487e461e1b8101ff138c90f40c Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Thu, 14 Aug 2014 17:28:35 +0200 Subject: [PATCH] Detect staged packages in a more robust way. Instead of checking the review status of the request to detect if a package is in a staging project, we use the staging API knowledge about packages. This fix the problem that happends when a staged request is declined and reopened. --- osclib/request_finder.py | 36 ------------------------------------ osclib/select_command.py | 24 ++++++++++++++---------- 2 files changed, 14 insertions(+), 46 deletions(-) diff --git a/osclib/request_finder.py b/osclib/request_finder.py index 801ebc61..f77467b9 100644 --- a/osclib/request_finder.py +++ b/osclib/request_finder.py @@ -30,31 +30,6 @@ class RequestFinder(object): self.api = api self.srs = {} - def _filter_review_by_project(self, element, state): - """ - Takes a XML that contains a list of reviews and take the ones - that in state 'state'. - :param element: xml list with reviews - :param state: state we filter for - """ - reviews = [r.get('by_project') - for r in element.findall('review') - if r.get('by_project') and r.get('state') == state] - return reviews - - def _new_review_by_project(self, request_id, element): - """ - Takes a XML that contains a list of reviews and return the - staging project currantly assigned for review that is in 'new' - state. Makes sure that there is a most one. - :param request_id: request id - :param element: XML with list of reviews - """ - reviews = self._filter_review_by_project(element, 'new') - msg = 'Request "{}" have multiple review by project in new state "{}"'.format(request_id, reviews) - assert len(reviews) <= 1, msg - return reviews[0] if reviews else None - def find_request_id(self, request_id): """ Look up the request by ID to verify if it is correct @@ -82,10 +57,6 @@ class RequestFinder(object): raise oscerr.WrongArgs(msg) self.srs[int(request_id)] = {'project': project} - review = self._new_review_by_project(request_id, root) - if review and review.startswith('openSUSE:{}:Staging:'.format(self.api.opensuse)): - self.srs[int(request_id)]['staging'] = review - return True def find_request_package(self, package): @@ -113,10 +84,6 @@ class RequestFinder(object): self.srs[request] = {'project': 'openSUSE:{}'.format(self.api.opensuse), 'state': state} - review = self._new_review_by_project(request, sr) - if review and review.startswith('openSUSE:{}:Staging:'.format(self.api.opensuse)): - self.srs[int(request)]['staging'] = review - if last_rq: if self.srs[last_rq]['state'] == 'declined': # ignore previous requests if they are declined @@ -153,9 +120,6 @@ class RequestFinder(object): request = int(sr.attrib['id']) state = sr.find('state').get('name') self.srs[request] = {'project': 'openSUSE:{}'.format(self.api.opensuse), 'state': state} - review = self._new_review_by_project(request, sr) - if review and review.startswith('openSUSE:{}:Staging:'.format(self.api.opensuse)): - self.srs[int(request)]['staging'] = review ret = True return ret diff --git a/osclib/select_command.py b/osclib/select_command.py index f3dbf9b9..3d7588c3 100644 --- a/osclib/select_command.py +++ b/osclib/select_command.py @@ -56,22 +56,26 @@ class SelectCommand(object): return candidates[0] if candidates else None - def select_request(self, request, request_project, move, from_): + def select_request(self, request, move, from_): supersede = self._supersede(request) - if 'staging' not in request_project and not supersede: + staged_requests = { + self.api.packages_staged[package]['rq_id']: package for package in self.api.packages_staged + } + + if request not in staged_requests and not supersede: # Normal 'select' command print('Adding request "{}" to project "{}"'.format(request, self.target_project)) return self.api.rq_to_prj(request, self.target_project) - elif 'staging' in request_project and (move or supersede): + elif request in staged_requests and (move or supersede): # 'select' command becomes a 'move' fprj = None if from_: fprj = self.api.prj_from_letter(from_) else: # supersede = (new_rq, package, project) - fprj = request_project['staging'] if not supersede else supersede[2] + fprj = staged_requests[request] if not supersede else supersede[2] if supersede: print('"{} ({}) is superseded by {}'.format(request, supersede[1], supersede[0])) @@ -86,13 +90,13 @@ class SelectCommand(object): self.affected_projects.add(fprj) return self.api.move_between_project(fprj, request, self.target_project) - elif 'staging' in request_project and not move: + elif request in staged_requests and not move: # Previously selected, but not explicit move msg = 'Request {} is already tracked in "{}".' - msg = msg.format(request, request_project['staging']) - if request_project['staging'] != self.target_project: + msg = msg.format(request, staged_requests[request]) + if staged_requests[request] != self.target_project: msg += '\nUse --move modifier to move the request from "{}" to "{}"' - msg = msg.format(request_project['staging'], self.target_project) + msg = msg.format(staged_requests[request], self.target_project) print(msg) return True else: @@ -114,8 +118,8 @@ class SelectCommand(object): # FreezeCommand(self.api).perform(target_project) self.target_project = target_project - for request, request_project in RequestFinder.find_sr(requests, self.api).items(): - if not self.select_request(request, request_project, move, from_): + for request in RequestFinder.find_sr(requests, self.api): + if not self.select_request(request, move, from_): return False # Notify everybody about the changes