From ec6ed880f40e56140e88e41024428f9a8ac21265 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Sat, 6 Apr 2019 17:44:59 -0700 Subject: basic dummy review bot --- python/fatcat_review.py | 60 +++++++ python/fatcat_tools/reviewers/__init__.py | 2 + python/fatcat_tools/reviewers/review_common.py | 237 +++++++++++++++++++++++++ 3 files changed, 299 insertions(+) create mode 100755 python/fatcat_review.py create mode 100644 python/fatcat_tools/reviewers/__init__.py create mode 100644 python/fatcat_tools/reviewers/review_common.py diff --git a/python/fatcat_review.py b/python/fatcat_review.py new file mode 100755 index 00000000..35722972 --- /dev/null +++ b/python/fatcat_review.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 + +import sys +import argparse +import datetime +import raven + +from fatcat_tools import authenticated_api +from fatcat_tools.reviewers import DummyReviewBot, ExtIdReviewBot, ReviewBot + +# Yep, a global. Gets DSN from `SENTRY_DSN` environment variable +sentry_client = raven.Client() + + +def run_dummy(args): + reviewer = DummyReviewBot(args.api, poll_interval=args.poll_interval, + verbose=args.debug) + if args.editgroup: + annotation = reviewer.run_single(args.editgroup, args.annotate) + print(annotation) + elif args.continuous: + reviewer.run() + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('--debug', + action='store_true', + help="enable debug logging") + parser.add_argument('--api-host-url', + default="http://localhost:9411/v0", + help="fatcat API host/port to use") + parser.add_argument('--poll-interval', + help="how long to wait between polling (seconds)", + default=10.0, type=float) + subparsers = parser.add_subparsers() + + sub_dummy = subparsers.add_parser('dummy') + sub_dummy.set_defaults(func=run_dummy) + sub_dummy.add_argument("--continuous", + action="store_true", + help="run forever, polling for new reviewable editgroups") + sub_dummy.add_argument("--editgroup", + help="single editgroup ID to review") + sub_dummy.add_argument("--annotate", + action="store_true", + help="for single editgroups, pushes result as annotation") + + args = parser.parse_args() + if not args.__dict__.get("func"): + print("tell me what to do!") + sys.exit(-1) + if (args.editgroup and args.continuous) or not (args.editgroup or args.continuous): + print("need to run on a single editgroup, or continuous") + sys.exit(-1) + + args.api = authenticated_api(args.api_host_url) + args.func(args) + +if __name__ == '__main__': + main() diff --git a/python/fatcat_tools/reviewers/__init__.py b/python/fatcat_tools/reviewers/__init__.py new file mode 100644 index 00000000..edd56e9f --- /dev/null +++ b/python/fatcat_tools/reviewers/__init__.py @@ -0,0 +1,2 @@ + +from .review_common import ReviewBot, EditCheck, CheckResult, DummyReviewBot diff --git a/python/fatcat_tools/reviewers/review_common.py b/python/fatcat_tools/reviewers/review_common.py new file mode 100644 index 00000000..239ce209 --- /dev/null +++ b/python/fatcat_tools/reviewers/review_common.py @@ -0,0 +1,237 @@ + +import json +import time +import datetime +import subprocess +from collections import Counter + +import fatcat_client +from fatcat_client.rest import ApiException + +""" +checks should return: +- status: pass, fail, warning, skipped +- description + +annotation extra should include: +- useragent +- git_rev +- submit timestamp + +can apply to individual entity revs, or to entire editgroup + +reviewbot takes an editgroup (object) and returns an annotation (object) +""" + + +class CheckResult: + status = None + description = None + ident = None + rev = None + check_type = None + + def __init__(self, status, check_type=None, description=None, **kwargs): + self.status = status + self.check_type = check_type + self.description = description + self.ident = kwargs.get('ident') + self.rev = kwargs.get('rev') + + def __repr__(self): + return str(self.__dict__) + + +class EditCheck: + + scope = [] + name = None + + def check_editgroup(self, editgroup): + raise NotImplementedError + + def check_container(self, edit, entity): + raise NotImplementedError + + def check_creator(self, edit, entity): + raise NotImplementedError + + def check_file(self, edit, entity): + raise NotImplementedError + + def check_fileset(self, edit, entity): + raise NotImplementedError + + def check_webcapture(self, edit, entity): + raise NotImplementedError + + def check_release(self, edit, entity): + raise NotImplementedError + + def check_work(self, edit, work): + raise NotImplementedError + + +class ReviewBot: + + def __init__(self, api, verbose=False, **kwargs): + + self.api = api + self.checks = [] + self.verbose = verbose + self.extra = kwargs.get('extra', dict()) + self.extra['git_rev'] = self.extra.get('git_rev', + subprocess.check_output(["git", "describe", "--always"]).strip()).decode('utf-8') + self.extra['agent'] = self.extra.get('agent', 'fatcat_tools.ReviewBot') + self.poll_interval = kwargs.get('poll_interval', 10.0) + + def run_single(self, editgroup_id, annotate=True): + eg = self.api.get_editgroup(editgroup_id) + annotation = self.review_editgroup(eg) + if annotate: + api.create_editgroup_annotation(eg.editgroup_id, annotation) + return annotation + + def run(self, since=None): + if since == None: + since = datetime.datetime.utcnow() + while True: + # XXX: better isoformat conversion? + eg_list = self.api.get_editgroups_reviewable(since=since.isoformat()[:19] + "Z", limit=100) + if not eg_list: + print("Sleeping {} seconds...".format(self.poll_interval)) + time.sleep(self.poll_interval) + continue + for eg in eg_list: + # TODO: fetch annotations to ensure we haven't already annotated + annotation = self.review_editgroup(eg) + print("Reviewed {} disposition:{}".format( + eg.editgroup_id, annotation.extra['disposition'])) + self.api.create_editgroup_annotation(eg.editgroup_id, annotation) + since = eg.submitted + # to prevent busy loops (TODO: needs review/rethink; multiple + # editgroups in the same second) + since = since + datetime.timedelta(seconds=1) + + def review_editgroup(self, editgroup): + results = self.run_checks(editgroup) + result_counts = self.result_counts(results) + disposition = self.disposition(results) + if disposition == "accept": + comment = "This editgroup looks great! All checks passed." + elif disposition == "revise": + comment = "Some issues were found, and changes or close review are recommended before accepting." + elif disposition == "reject": + comment = "Serious issues were found; this editgroup should **not** be accepted." + else: + raise ValueError + + for (status, title) in (('fail', 'Failed check'), ('warning', 'Warnings')): + if result_counts[status] > 0: + comment += "\n\n### {} ({}):\n".format( + status, result_counts[status]) + for result in results: + if result.status == status and result.check_type == "editgroup": + comment += "\n- {description}".format(result.description) + if result.status == status and result.check_type != "editgroup": + comment += "\n- {check_type} [{rev}](/{release_type}/rev/{rev}): {description}".format( + check_type=result.check_type, + rev=result.rev, + description=result.description) + + extra = self.extra.copy() + extra.update({ + "disposition": disposition, + "submit_timestamp": editgroup.submitted.isoformat(), + "checks": [check.name for check in self.checks], + "result_counts": dict(result_counts), + }) + annotation = fatcat_client.EditgroupAnnotation( + comment_markdown=comment, + editgroup_id=editgroup.editgroup_id, + extra=extra, + ) + return annotation + + def result_counts(self, results): + counts = Counter() + for result in results: + counts['total'] += 1 + counts[result.status] += 1 + return counts + + def disposition(self, results): + """ + Returns one of: accept, revise, reject + """ + raise NotImplementedError + + def run_checks(self, editgroup): + + results = [] + + # any full-editgroup checks + for check in self.checks: + if "editgroup" in check.scope: + result = check.check_editgroup(editgroup) + if self.verbose: + print(result) + results.append(result) + + if not editgroup.edits: + entity_edits = {} + else: + entity_edits = { + "container": editgroup.edits.containers, + "creator": editgroup.edits.creators, + "file": editgroup.edits.files, + "fileset": editgroup.edits.filesets, + "webcapture": editgroup.edits.webcaptures, + "release": editgroup.edits.releases, + "work": editgroup.edits.works, + } + + # entity-specific checks + for entity_type, edits in entity_edits.items(): + for edit in edits: + entity = None + for check in self.checks: + if entity_type in check.scope: + # hack-y python munging + get_method = getattr(api, "get_{}_rev".format(entity_type)) + check_method = getattr(check, "check_{}".format(entity_type)) + entity = get_method(api, edit.rev) + result = check_method(check, edit, entity) + result.rev = edit.rev + result.ident = edit.ident + if self.verbose: + print(result) + results.append(result) + + return results + + +class DummyCheck(EditCheck): + + scope = ["editgroup", "work"] + name = "DummyCheck" + + def check_editgroup(self, editgroup): + return CheckResult("pass", "editgroup", + "every edit is precious, thanks [editor {editor_id}](/editor/{editor_id})!".format( + editor_id=editgroup.editor_id)) + + def check_work(self, entity, edit): + return CheckResult("pass", "work", "this work edit is beautiful") + +class DummyReviewBot(ReviewBot): + """ + This bot reviews everything and always passes. + """ + + def __init__(self, api, **kwargs): + super().__init__(api, **kwargs) + self.checks = [DummyCheck()] + + def disposition(self, results): + return "accept" -- cgit v1.2.3 From c8a422d5c3e9b8153dbb1df12aaafcc0c9c9c9c8 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Sat, 6 Apr 2019 19:20:40 -0700 Subject: fix bad review import --- python/fatcat_review.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/fatcat_review.py b/python/fatcat_review.py index 35722972..40bc7041 100755 --- a/python/fatcat_review.py +++ b/python/fatcat_review.py @@ -6,7 +6,7 @@ import datetime import raven from fatcat_tools import authenticated_api -from fatcat_tools.reviewers import DummyReviewBot, ExtIdReviewBot, ReviewBot +from fatcat_tools.reviewers import DummyReviewBot, ReviewBot # Yep, a global. Gets DSN from `SENTRY_DSN` environment variable sentry_client = raven.Client() -- cgit v1.2.3 From 378947eb098c8e2811a70fb5921ef034a6bca4ca Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Sat, 6 Apr 2019 19:21:10 -0700 Subject: fix reviewer bugs (thanks pylint) --- python/fatcat_tools/reviewers/review_common.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/fatcat_tools/reviewers/review_common.py b/python/fatcat_tools/reviewers/review_common.py index 239ce209..bb36d4e3 100644 --- a/python/fatcat_tools/reviewers/review_common.py +++ b/python/fatcat_tools/reviewers/review_common.py @@ -89,7 +89,7 @@ class ReviewBot: eg = self.api.get_editgroup(editgroup_id) annotation = self.review_editgroup(eg) if annotate: - api.create_editgroup_annotation(eg.editgroup_id, annotation) + self.api.create_editgroup_annotation(eg.editgroup_id, annotation) return annotation def run(self, since=None): @@ -198,9 +198,9 @@ class ReviewBot: for check in self.checks: if entity_type in check.scope: # hack-y python munging - get_method = getattr(api, "get_{}_rev".format(entity_type)) + get_method = getattr(self.api, "get_{}_rev".format(entity_type)) check_method = getattr(check, "check_{}".format(entity_type)) - entity = get_method(api, edit.rev) + entity = get_method(self.api, edit.rev) result = check_method(check, edit, entity) result.rev = edit.rev result.ident = edit.ident -- cgit v1.2.3 From 3caa8905ef2f413ec8fa107dd54f03356f4052d8 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Sat, 6 Apr 2019 19:21:52 -0700 Subject: remove title from metadata macro --- python/fatcat_web/templates/container_view.html | 1 + python/fatcat_web/templates/creator_view.html | 1 + python/fatcat_web/templates/entity_macros.html | 1 - python/fatcat_web/templates/file_view.html | 1 + python/fatcat_web/templates/fileset_view.html | 1 + python/fatcat_web/templates/release_view.html | 1 + python/fatcat_web/templates/webcapture_view.html | 1 + python/fatcat_web/templates/work_view.html | 1 + 8 files changed, 7 insertions(+), 1 deletion(-) diff --git a/python/fatcat_web/templates/container_view.html b/python/fatcat_web/templates/container_view.html index 3d0627ca..619bd647 100644 --- a/python/fatcat_web/templates/container_view.html +++ b/python/fatcat_web/templates/container_view.html @@ -41,6 +41,7 @@ {% if entity.extra %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} diff --git a/python/fatcat_web/templates/creator_view.html b/python/fatcat_web/templates/creator_view.html index 8f61d74b..63308a51 100644 --- a/python/fatcat_web/templates/creator_view.html +++ b/python/fatcat_web/templates/creator_view.html @@ -32,6 +32,7 @@ {% endif %} {% if entity.extra != None %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} diff --git a/python/fatcat_web/templates/entity_macros.html b/python/fatcat_web/templates/entity_macros.html index 6d9ceed0..90c8e952 100644 --- a/python/fatcat_web/templates/entity_macros.html +++ b/python/fatcat_web/templates/entity_macros.html @@ -26,7 +26,6 @@ {% macro extra_metadata(extra) -%} -

Extra Metadata (raw JSON)

{% for (key, value) in extra.items() %} diff --git a/python/fatcat_web/templates/file_view.html b/python/fatcat_web/templates/file_view.html index 7a428b55..097ee5f6 100644 --- a/python/fatcat_web/templates/file_view.html +++ b/python/fatcat_web/templates/file_view.html @@ -17,6 +17,7 @@
{% if entity.extra %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} diff --git a/python/fatcat_web/templates/fileset_view.html b/python/fatcat_web/templates/fileset_view.html index ada2ae51..7f41e617 100644 --- a/python/fatcat_web/templates/fileset_view.html +++ b/python/fatcat_web/templates/fileset_view.html @@ -17,6 +17,7 @@
{% if entity.extra %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} diff --git a/python/fatcat_web/templates/release_view.html b/python/fatcat_web/templates/release_view.html index ff044c49..4849842c 100644 --- a/python/fatcat_web/templates/release_view.html +++ b/python/fatcat_web/templates/release_view.html @@ -191,6 +191,7 @@ {% if entity.extra %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} diff --git a/python/fatcat_web/templates/webcapture_view.html b/python/fatcat_web/templates/webcapture_view.html index 921d5d48..1dd179f6 100644 --- a/python/fatcat_web/templates/webcapture_view.html +++ b/python/fatcat_web/templates/webcapture_view.html @@ -21,6 +21,7 @@ {% if entity.extra %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} diff --git a/python/fatcat_web/templates/work_view.html b/python/fatcat_web/templates/work_view.html index 7842ec50..507498c8 100644 --- a/python/fatcat_web/templates/work_view.html +++ b/python/fatcat_web/templates/work_view.html @@ -17,6 +17,7 @@
{% if entity.extra != None %} +

Extra Metadata (raw JSON)

{{ entity_macros.extra_metadata(entity.extra) }} {% endif %} -- cgit v1.2.3 From d89787882aaf4b7342768ac56d2252f3fcb7a7ad Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Sat, 6 Apr 2019 19:22:58 -0700 Subject: diplay extra metadata, disposition in annotations --- python/fatcat_web/templates/editgroup_view.html | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/python/fatcat_web/templates/editgroup_view.html b/python/fatcat_web/templates/editgroup_view.html index f7f3ad45..9d152579 100644 --- a/python/fatcat_web/templates/editgroup_view.html +++ b/python/fatcat_web/templates/editgroup_view.html @@ -27,6 +27,8 @@ Revision: {{ edit.revision }} {% endif %} {% if edit.extra %} + Extra Metadata (raw JSON) + {{ entity_macros.extra_metadata(edit.extra) }} {% endif %}
@@ -90,6 +92,10 @@ reviewable bundle. {% else %} none {% endif %} +{% if editgroup.extra %} +

Extra Metadata (raw JSON)

+ {{ entity_macros.extra_metadata(editgroup.extra) }} +{% endif %}

@@ -117,11 +123,23 @@ reviewable bundle. Admin {% endif %} at {{ annotation.created.strftime("%Y-%m-%d %H:%M:%S") }} + {# TODO: get individual editgroup annotation not supported yet + (as JSON) + #} + {% if annotation.extra and annotation.extra.disposition %} + {% set disp = annotation.extra.disposition %} + {{ annotation.extra.disposition|capitalize }} + {% endif %}
{% if annotation.extra %} -
- {{ entity_macros.extra_metadata(annotation.extra) }} -
+
+
+ Review Metadata (raw JSON) +
+
+ {{ entity_macros.extra_metadata(annotation.extra) }} +
+
{% endif %}
{{ annotation.comment_markdown|markdown(escape=True) }} -- cgit v1.2.3