This is an automated email from the ASF dual-hosted git repository. andschwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 590a75d0c9d61b0b07f8a3807225c40eb8189a0b Author: Dragos Schebesch <[email protected]> AuthorDate: Tue Aug 21 10:42:52 2018 -0700 Refactored verify-reviews.py to use commons.py and argparse. Review: https://reviews.apache.org/r/67505/ --- support/python3/verify-reviews.py | 241 +++++++++++++++++--------------------- 1 file changed, 110 insertions(+), 131 deletions(-) diff --git a/support/python3/verify-reviews.py b/support/python3/verify-reviews.py index 2e92590..14e5f9b 100755 --- a/support/python3/verify-reviews.py +++ b/support/python3/verify-reviews.py @@ -33,7 +33,9 @@ The script performs the following sequence: * The result is posted to ReviewBoard. """ -import atexit +import argparse +import time +import datetime import json import os import platform @@ -43,39 +45,46 @@ import urllib.error import urllib.parse import urllib.request -from datetime import datetime +from common import ReviewBoardHandler, ReviewError, REVIEWBOARD_URL -REVIEWBOARD_URL = "https://reviews.apache.org"; REVIEW_SIZE = 1000000 # 1 MB in bytes. - -# TODO(vinod): Use 'argparse' module. -# Get the user and password from command line. -if len(sys.argv) < 3: - print("Usage: ./verify-reviews.py <user>" - "<password> [num-reviews] [query-params]") - sys.exit(1) - -USER = sys.argv[1] -PASSWORD = sys.argv[2] - -# Number of reviews to verify. -NUM_REVIEWS = -1 # All possible reviews. -if len(sys.argv) >= 4: - NUM_REVIEWS = int(sys.argv[3]) - -# Unless otherwise specified consider pending review requests to Mesos updated -# since 03/01/2014. -GROUP = "mesos" -LAST_UPDATED = "2014-03-01T00:00:00" -QUERY_PARAMS = "?to-groups=%s&status=pending&last-updated-from=%s" \ - % (GROUP, LAST_UPDATED) -if len(sys.argv) >= 5: - QUERY_PARAMS = sys.argv[4] - - -class ReviewError(Exception): - """Exception returned by post_review().""" - pass +# This is the mesos repo ID obtained from querying the reviews.apache.org API +MESOS_REPOSITORY_ID = 122 + + +def parse_parameters(): + """Method to parse the arguments for argparse.""" + parser = argparse.ArgumentParser( + description="Reviews that need verification from the Review Board") + parser.add_argument("-u", "--user", type=str, required=True, + help="Review Board user name") + parser.add_argument("-p", "--password", type=str, required=True, + help="Review Board user password") + parser.add_argument("-r", "--reviews", type=int, required=False, + default=-1, help="The number of reviews to fetch," + " that will need verification") + default_hours_behind = 8 + datetime_before = (datetime.datetime.now() - + datetime.timedelta(hours=default_hours_behind)) + datetime_before_string = datetime_before.isoformat() + default_query = {"status": "pending", "repository": MESOS_REPOSITORY_ID, + "last-updated-from": datetime_before_string.split(".")[0]} + parser.add_argument("-q", "--query", type=str, required=False, + help="Query parameters, passed as string in JSON" + " format. Example: '%s'" % json.dumps( + default_query), + default=json.dumps(default_query)) + + subparsers = parser.add_subparsers(title="The script plug-in type") + + file_parser = subparsers.add_parser( + "file", description="File plug-in just writes to a file all" + " the review ids that need verification") + file_parser.add_argument("-o", "--out-file", type=str, required=True, + help="The out file with the reviews IDs that" + " need verification") + + return parser.parse_args() def shell(command): @@ -85,38 +94,13 @@ def shell(command): command, stderr=subprocess.STDOUT, shell=True) -HEAD = shell("git rev-parse HEAD") - - -def api(url, data=None): - """Call the ReviewBoard API.""" - try: - auth_handler = urllib.request.HTTPBasicAuthHandler() - auth_handler.add_password( - realm="Web API", - uri="reviews.apache.org", - user=USER, - passwd=PASSWORD) - - opener = urllib.request.build_opener(auth_handler) - urllib.request.install_opener(opener) - - return json.loads(urllib.request.urlopen(url, data=data).read()) - except urllib.error.HTTPError as err: - print("Error handling URL %s: %s (%s)" % (url, err.reason, err.read())) - exit(1) - except urllib.error.URLError as err: - print("Error handling URL %s: %s" % (url, err.reason)) - exit(1) - - def apply_review(review_id): """Apply a review using the script apply-reviews.py.""" print("Applying review %s" % review_id) shell("python support/python3/apply-reviews.py -n -r %s" % review_id) -def apply_reviews(review_request, reviews): +def apply_reviews(review_request, reviews, handler): """Apply multiple reviews at once.""" # If there are no reviewers specified throw an error. if not review_request["target_people"]: @@ -134,34 +118,37 @@ def apply_reviews(review_request, reviews): # First recursively apply the dependent reviews. for review in review_request["depends_on"]: review_url = review["href"] - print("Dependent review: %s " % review_url) - apply_reviews(api(review_url)["review_request"], reviews) + print("Dependent review: %s" % review_url) + apply_reviews(handler.api(review_url)["review_request"], + reviews, handler) # Now apply this review if not yet submitted. if review_request["status"] != "submitted": apply_review(review_request["id"]) -def post_review(review_request, message): +def post_review(review_request, message, handler): """Post a review on the review board.""" print("Posting review: %s" % message) review_url = review_request["links"]["reviews"]["href"] data = urllib.parse.urlencode({'body_top': message, 'public': 'true'}) - api(review_url, data) + handler.api(review_url, data) [email protected] +# @atexit.register def cleanup(): """Clean the git repository.""" try: shell("git clean -fd") - shell("git reset --hard %s" % HEAD) + HEAD = shell("git rev-parse HEAD") + print(HEAD) + shell("git checkout HEAD -- %s" % HEAD) except subprocess.CalledProcessError as err: print("Failed command: %s\n\nError: %s" % (err.cmd, err.output)) -def verify_review(review_request): +def verify_review(review_request, handler): """Verify a review.""" print("Verifying review %s" % review_request["id"]) build_output = "build_" + str(review_request["id"]) @@ -169,11 +156,10 @@ def verify_review(review_request): try: # Recursively apply the review and its dependents. reviews = [] - apply_reviews(review_request, reviews) + apply_reviews(review_request, reviews, handler) reviews.reverse() # Reviews are applied in the reverse order. - command = "" if platform.system() == 'Windows': command = "support\\windows-build.bat" @@ -199,15 +185,15 @@ def verify_review(review_request): # output. `pipefail` ensures that the exit status of the build # command ispreserved even after tee'ing. subprocess.check_call(['bash', '-c', - ('set -o pipefail; %s 2>&1 | tee %s') + 'set -o pipefail; %s 2>&1 | tee %s' % (command, build_output)]) # Success! post_review( review_request, - "Patch looks great!\n\n" \ - "Reviews applied: %s\n\n" \ - "Passed command: %s" % (reviews, command)) + "Patch looks great!\n\n" + "Reviews applied: %s\n\n" + "Passed command: %s" % (reviews, command), handler) except subprocess.CalledProcessError as err: # If we are here because the docker build command failed, read the # output from `build_output` file. For all other command failures read @@ -239,80 +225,73 @@ def verify_review(review_request): "Bad patch!\n\n" \ "Reviews applied: %s\n\n" \ "Failed command: %s\n\n" \ - "Error:\n%s" % (reviews, err.cmd, output)) + "Error:\n%s" % (reviews, err.cmd, output), handler) except ReviewError as err: post_review( review_request, "Bad review!\n\n" \ "Reviews applied: %s\n\n" \ - "Error:\n%s" % (reviews, err.args[0])) + "Error:\n%s" % (reviews, err.args[0]), handler) # Clean up. - cleanup() - - -def needs_verification(review_request): - """Return True if this review request needs to be verified.""" - print("Checking if review: %s needs verification" % review_request["id"]) - - # Skip if the review blocks another review. - if review_request["blocks"]: - print("Skipping blocking review %s" % review_request["id"]) - return False - - diffs_url = review_request["links"]["diffs"]["href"] - diffs = api(diffs_url) - - if not diffs["diffs"]: # No diffs attached! - print("Skipping review %s as it has no diffs" % review_request["id"]) - return False - - # Get the timestamp of the latest diff. - timestamp = diffs["diffs"][-1]["timestamp"] - rb_date_format = "%Y-%m-%dT%H:%M:%SZ" - diff_time = datetime.strptime(timestamp, rb_date_format) - print("Latest diff timestamp: %s" % diff_time) - - # Get the timestamp of the latest review from this script. - reviews_url = review_request["links"]["reviews"]["href"] - reviews = api(reviews_url + "?max-results=200") - review_time = None - for review in reversed(reviews["reviews"]): - if review["links"]["user"]["title"] == USER: - timestamp = review["timestamp"] - review_time = datetime.strptime(timestamp, rb_date_format) - print("Latest review timestamp: %s" % review_time) - break - - # TODO: Apply this check recursively up the dependency chain. - changes_url = review_request["links"]["changes"]["href"] - changes = api(changes_url) - dependency_time = None - for change in changes["changes"]: - if "depends_on" in change["fields_changed"]: - timestamp = change["timestamp"] - dependency_time = datetime.strptime(timestamp, rb_date_format) - print("Latest dependency change timestamp: %s" % dependency_time) - break - - # Needs verification if there is a new diff, or if the dependencies changed, - # after the last time it was verified. - return not review_time or review_time < diff_time or \ - (dependency_time and review_time < dependency_time) + # cleanup() + + +def verification_needed_write(review_ids, parameters): + """Write the IDs of the review requests that need verification.""" + num_reviews = len(review_ids) + print("%s review requests need verification" % num_reviews) + # out_file parameter is mandatory to be passed + try: + # Using file plug-in + with open(parameters.out_file, 'w') as f: + f.write('\n'.join(review_ids)) + except Exception: + print("Failed opening file '%s' for writing" % parameters.out_file) + raise def main(): """Main function to verify the submitted reviews.""" - review_requests_url = \ - "%s/api/review-requests/%s" % (REVIEWBOARD_URL, QUERY_PARAMS) - - review_requests = api(review_requests_url) + parameters = parse_parameters() + print("\n%s - Running %s" % (time.strftime('%m-%d-%y_%T'), + os.path.abspath(__file__))) + # The colon from timestamp gets encoded and we don't want it to be encoded. + # Replacing %3A with colon. + query_string = urllib.parse.urlencode( + json.loads(parameters.query)).replace("%3A", ":") + review_requests_url = "%s/api/review-requests/?%s" % (REVIEWBOARD_URL, + query_string) + handler = ReviewBoardHandler(parameters.user, parameters.password) num_reviews = 0 + review_ids = [] + review_requests = handler.api(review_requests_url) for review_request in reversed(review_requests["review_requests"]): - if (NUM_REVIEWS == -1 or num_reviews < NUM_REVIEWS) and \ - needs_verification(review_request): - verify_review(review_request) + if parameters.reviews == -1 or num_reviews < parameters.reviews: + try: + needs_verification = handler.needs_verification(review_request) + if not needs_verification: + continue + # An exception is raised if cyclic dependencies are found + handler.get_dependent_review_ids(review_request) + except ReviewError as err: + message = ("Bad review!\n\n" + "Error:\n%s" % (err.args[0])) + handler.post_review(review_request, message, handler) + continue + except Exception as err: + print("Error occured: %s" % err) + needs_verification = False + print("WARNING: Cannot find if review %s needs" + " verification" % (review_request["id"])) + if not needs_verification: + continue + review_ids.append(str(review_request["id"])) num_reviews += 1 + verify_review(review_request, handler) + + verification_needed_write(review_ids, parameters) + if __name__ == '__main__': main()
