You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/02/20 00:03:56 UTC

mesos git commit: Fixed ReviewBot to catch circular dependencies in review requests.

Repository: mesos
Updated Branches:
  refs/heads/master e12fecc3b -> a27b394ec


Fixed ReviewBot to catch circular dependencies in review requests.

Review: https://reviews.apache.org/r/43747


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a27b394e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a27b394e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a27b394e

Branch: refs/heads/master
Commit: a27b394ecc9764966f2571c89b954e74df21b026
Parents: e12fecc
Author: Vinod Kone <vi...@gmail.com>
Authored: Thu Feb 18 14:45:34 2016 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Feb 19 15:03:35 2016 -0800

----------------------------------------------------------------------
 support/verify_reviews.py | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a27b394e/support/verify_reviews.py
----------------------------------------------------------------------
diff --git a/support/verify_reviews.py b/support/verify_reviews.py
index f0b9996..208a078 100755
--- a/support/verify_reviews.py
+++ b/support/verify_reviews.py
@@ -79,25 +79,29 @@ def apply_review(review_id):
     shell("./support/apply-review.sh -n -r %s" % review_id)
 
 
-def apply_reviews(review_request, applied):
-    # Skip this review if it's already applied.
-    if review_request["id"] in applied:
-        print "Skipping already applied review %s" % review_request["id"]
-
+def apply_reviews(review_request, reviews):
+    # If there are no reviewers specified throw an error.
     if not review_request["target_people"]:
-      raise ReviewError("No reviewers specified. Please find a reviewer by"
-                        " asking on JIRA or the mailing list.")
+        raise ReviewError("No reviewers specified. Please find a reviewer by"
+                          " asking on JIRA or the mailing list.")
+
+    # If there is a circular dependency throw an error.`
+    if review_request["id"] in reviews:
+        raise ReviewError("Circular dependency detected for review %s."
+                          "Please fix the 'depends_on' field."
+                          % review_request["id"])
+    else:
+        reviews.append(review_request["id"])
 
     # 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"], applied)
+        apply_reviews(api(review_url)["review_request"], reviews)
 
     # Now apply this review if not yet submitted.
-    applied.append(review_request["id"])
     if review_request["status"] != "submitted":
-      apply_review(review_request["id"])
+        apply_review(review_request["id"])
 
 
 def post_review(review_request, message):
@@ -123,8 +127,10 @@ def verify_review(review_request):
 
     try:
         # Recursively apply the review and its dependents.
-        applied = []
-        apply_reviews(review_request, applied)
+        reviews = []
+        apply_reviews(review_request, reviews)
+
+        reviews.reverse() # Reviews are applied in the reverse order.
 
         # Launch docker build script.
 
@@ -150,7 +156,7 @@ def verify_review(review_request):
             review_request,
             "Patch looks great!\n\n" \
             "Reviews applied: %s\n\n" \
-            "Passed command: %s" % (applied, command))
+            "Passed command: %s" % (reviews, command))
     except subprocess.CalledProcessError as e:
         # If we are here because the docker build command failed, read the
         # output from `build_output` file. For all other command failures read
@@ -166,13 +172,13 @@ def verify_review(review_request):
             "Bad patch!\n\n" \
             "Reviews applied: %s\n\n" \
             "Failed command: %s\n\n" \
-            "Error:\n%s" % (applied, e.cmd, output))
+            "Error:\n%s" % (reviews, e.cmd, output))
     except ReviewError as e:
         post_review(
             review_request,
             "Bad review!\n\n" \
             "Reviews applied: %s\n\n" \
-            "Error:\n%s" % (applied, e.args[0]))
+            "Error:\n%s" % (reviews, e.args[0]))
 
     # Clean up.
     cleanup()