diff --git a/bugwarrior/collect.py b/bugwarrior/collect.py index 99755e83..c0fd163e 100644 --- a/bugwarrior/collect.py +++ b/bugwarrior/collect.py @@ -38,7 +38,7 @@ def _aggregate_issues(conf, main_section, target, queue): issue_count += 1 except SystemExit as e: log.critical(f"Worker for [{target}] exited: {e}") - queue.put((SERVICE_FINISHED_ERROR, (target, e))) + queue.put((SERVICE_FINISHED_ERROR, target)) except BaseException as e: if hasattr(e, 'request') and e.request: # Exceptions raised by requests library have the HTTP request @@ -47,10 +47,10 @@ def _aggregate_issues(conf, main_section, target, queue): # methods. There is no one left to call these hooks anyway. e.request.hooks = {} log.exception(f"Worker for [{target}] failed: {e}") - queue.put((SERVICE_FINISHED_ERROR, (target, e))) + queue.put((SERVICE_FINISHED_ERROR, target)) else: log.debug(f"Worker for [{target}] finished ok.") - queue.put((SERVICE_FINISHED_OK, (target, issue_count, ))) + queue.put((SERVICE_FINISHED_OK, target)) finally: duration = time.time() - start log.info(f"Done with [{target}] in {duration}.") @@ -87,14 +87,16 @@ def aggregate_issues(conf, main_section, debug): currently_running = len(targets) while currently_running > 0: issue = queue.get(True) - if isinstance(issue, tuple): - currently_running -= 1 - completion_type, args = issue - if completion_type == SERVICE_FINISHED_ERROR: - target, e = args - log.error(f"Aborted [{target}] due to critical error.") - yield ('SERVICE FAILED', target) - continue - yield issue + try: + yield issue.get_taskwarrior_record() + except AttributeError: + if isinstance(issue, tuple): + currently_running -= 1 + completion_type, target = issue + if completion_type == SERVICE_FINISHED_ERROR: + log.error(f"Aborted [{target}] due to critical error.") + yield ('SERVICE FAILED', target) + continue + yield issue log.info("Done aggregating remote issues.") diff --git a/bugwarrior/db.py b/bugwarrior/db.py index 16b8b327..f965525e 100644 --- a/bugwarrior/db.py +++ b/bugwarrior/db.py @@ -98,19 +98,11 @@ def get_managed_task_uuids(tw, key_list): return expected_task_ids -def make_unique_identifier(keys, issue): +def make_unique_identifier(keys: dict, issue: dict) -> str: """ For a given issue, make an identifier from its unique keys. This is not the same as the taskwarrior uuid, which is assigned only once the task is created. - - :params: - * `keys`: A list of lists of keys to use for uniquely identifying - an issue. - * `issue`: An instance of a subclass of `bugwarrior.services.Issue`. - - :returns: - * A single string UUID. """ for service, key_list in keys.items(): if all([key in issue for key in key_list]): @@ -326,17 +318,11 @@ def synchronize(issue_generator, conf, main_section, dry_run=False): 'closed': [], } - issue_map = {} # unique identifier -> issue dict + issue_map = {} # unique identifier -> issue for issue in issue_generator: - - try: - issue_dict = dict(issue) - except ValueError: - if isinstance(issue, tuple) and issue[0] == 'SERVICE FAILED': - targets.remove(issue[1]) - continue - else: - raise + if isinstance(issue, tuple) and issue[0] == 'SERVICE FAILED': + targets.remove(issue[1]) + continue # De-duplicate issues coming in unique_identifier = make_unique_identifier(key_list, issue) @@ -344,36 +330,35 @@ def synchronize(issue_generator, conf, main_section, dry_run=False): log.debug( f"Merging tags and skipping. Seen {unique_identifier} of {issue}") # Merge and deduplicate tags. - issue_map[unique_identifier]['tags'] += issue_dict['tags'] + issue_map[unique_identifier]['tags'] += issue['tags'] issue_map[unique_identifier]['tags'] = list(set(issue_map[unique_identifier]['tags'])) else: - issue_map[unique_identifier] = issue_dict + issue_map[unique_identifier] = issue seen_uuids = set() - for issue_dict in issue_map.values(): - + for issue in issue_map.values(): # We received this issue from The Internet, but we're not sure what # kind of encoding the service providers may have handed us. Let's try # and decode all byte strings from UTF8 off the bat. If we encounter # other encodings in the wild in the future, we can revise the handling # here. https://github.com/ralphbean/bugwarrior/issues/350 - for key in issue_dict.keys(): - if isinstance(issue_dict[key], bytes): + for key in issue.keys(): + if isinstance(issue[key], bytes): try: - issue_dict[key] = issue_dict[key].decode('utf-8') + issue[key] = issue[key].decode('utf-8') except UnicodeDecodeError: log.warn("Failed to interpret %r as utf-8" % key) # Blank priority should mean *no* priority - if issue_dict['priority'] == '': - issue_dict['priority'] = None + if issue['priority'] == '': + issue['priority'] = None try: - existing_taskwarrior_uuid = find_taskwarrior_uuid(tw, key_list, issue_dict) + existing_taskwarrior_uuid = find_taskwarrior_uuid(tw, key_list, issue) except MultipleMatches as e: log.exception("Multiple matches: %s", str(e)) except NotFound: # Create new task - issue_updates['new'].append(issue_dict) + issue_updates['new'].append(issue) else: # Update existing task. seen_uuids.add(existing_taskwarrior_uuid) _, task = tw.get_task(uuid=existing_taskwarrior_uuid) @@ -386,23 +371,23 @@ def synchronize(issue_generator, conf, main_section, dry_run=False): # Drop static fields from the upstream issue. We don't want to # overwrite local changes to fields we declare static. for field in main_config.static_fields: - if field in issue_dict: - del issue_dict[field] + if field in issue: + del issue[field] # Merge annotations & tags from online into our task object if main_config.merge_annotations: - merge_left('annotations', task, issue_dict, hamming=True) + merge_left('annotations', task, issue, hamming=True) if main_config.merge_tags: if main_config.replace_tags: - replace_left('tags', task, issue_dict, main_config.static_tags) + replace_left('tags', task, issue, main_config.static_tags) else: - merge_left('tags', task, issue_dict) + merge_left('tags', task, issue) - issue_dict.pop('annotations', None) - issue_dict.pop('tags', None) + issue.pop('annotations', None) + issue.pop('tags', None) - task.update(issue_dict) + task.update(issue) if task.get_changes(keep=True): issue_updates['changed'].append(task) diff --git a/bugwarrior/services/__init__.py b/bugwarrior/services/__init__.py index 10b9470c..c6d8f396 100644 --- a/bugwarrior/services/__init__.py +++ b/bugwarrior/services/__init__.py @@ -84,7 +84,7 @@ def get_owner(self, issue): @abc.abstractmethod def issues(self): - """ Returns a list of dicts representing issues from a remote service. + """ Returns a list of Issue instances representing issues from a remote service. Each item in the list should be a dict that looks something like this: @@ -196,7 +196,7 @@ def get_added_tags(self): return added_tags - def get_taskwarrior_record(self, refined=True): + def get_taskwarrior_record(self, refined=True) -> dict: if not getattr(self, '_taskwarrior_record', None): self._taskwarrior_record = self.to_taskwarrior() record = copy.deepcopy(self._taskwarrior_record) @@ -290,49 +290,6 @@ def refine_record(self, record): record[field] = getattr(self, 'get_default_%s' % field)() return record - def __iter__(self): - record = self.get_taskwarrior_record() - yield from record.keys() - - def keys(self): - return list(self.__iter__()) - - def iterkeys(self): - return self.__iter__() - - def items(self): - record = self.get_taskwarrior_record() - return list(record.items()) - - def iteritems(self): - record = self.get_taskwarrior_record() - yield from record.items() - - def update(self, *args): - raise AttributeError( - "You cannot set attributes on issues." - ) - - def get(self, attribute, default=None): - try: - return self[attribute] - except KeyError: - return default - - def __getitem__(self, attribute): - record = self.get_taskwarrior_record() - return record[attribute] - - def __setitem__(self, attribute, value): - raise AttributeError( - "You cannot set attributes on issues." - ) - - def __delitem__(self, attribute): - raise AttributeError( - "You cannot delete attributes from issues." - ) - @property def record(self): return self._foreign_record diff --git a/tests/test_command.py b/tests/test_command.py index ed799945..c4540143 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -17,18 +17,20 @@ def fake_github_issues(self): def fake_bz_issues(self): - yield from [{ - 'annotations': [], - 'bugzillabugid': 1234567, - 'bugzillastatus': 'NEW', - 'bugzillasummary': 'This is the issue summary', - 'bugzillaurl': 'https://http://one.com//show_bug.cgi?id=1234567', - 'bugzillaproduct': 'Product', - 'bugzillacomponent': 'Something', - 'description': '(bw)Is#1234567 - This is the issue summary .. https://http://one.com//show_bug.cgi?id=1234567', # noqa: E501 - 'priority': 'H', - 'project': 'Something', - 'tags': []}] + yield from [self.get_issue_for_record( + { + 'id': 1234567, + 'status': 'NEW', + 'summary': 'This is the issue summary', + 'product': 'Product', + 'component': 'Something', + 'description': '(bw)Is#1234567 - This is the issue summary .. https://http://one.com//show_bug.cgi?id=1234567', # noqa: E501 + 'priority': 'H', + 'project': 'Something', + 'tags': [] + }, { + 'url': 'https://http://one.com//show_bug.cgi?id=1234567', + })] class TestPull(ConfigTest): diff --git a/tests/test_db.py b/tests/test_db.py index d48ee36d..c79ed4c1 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -121,7 +121,7 @@ def get_tasks(tw): # These should be de-duplicated in db.synchronize before # writing out to taskwarrior. # https://github.com/ralphbean/bugwarrior/issues/601 - issue_generator = iter((issue, duplicate_issue,)) + issue_generator = iter((copy.deepcopy(issue), duplicate_issue,)) db.synchronize(issue_generator, bwconfig, 'general') self.assertEqual(get_tasks(tw), { @@ -144,7 +144,7 @@ def get_tasks(tw): # Change static field issue['project'] = 'other_project' - db.synchronize(iter((issue,)), bwconfig, 'general') + db.synchronize(iter((copy.deepcopy(issue),)), bwconfig, 'general') self.assertEqual(get_tasks(tw), { 'completed': [], @@ -182,7 +182,7 @@ def get_tasks(tw): 'pending': []}) # TEST REOPENED ISSUE - db.synchronize(iter((issue,)), bwconfig, 'general') + db.synchronize(iter((copy.deepcopy(issue),)), bwconfig, 'general') tasks = tw.load_tasks() self.assertEqual(