[WIP][SPARK-7018][Build]: Refactor dev/run-tests-jenkins into Python #7401

brennonyork · 2015-07-14T18:58:48Z

First draft, and WIP, of the refactoring of the run-tests-jenkins script into Python. Currently a few things are left out that, could and I think should, be smaller JIRA's after this.

There are still a few areas where we use environment variables where we don't need to (like CURRENT_BLOCK). I might get around to fixing this one in lieu of everything else, but wanted to point that out.
The PR tests are still written in bash. I opted to not change those and just rewrite the runner into Python. This is a great follow-on JIRA IMO.
All of the linting scripts are still in bash as well and would likely do to just add those in as follow-on JIRA's as well.

Still a WIP now, but would love to get initial rounds of feedback as we iterate on this / test with Jenkins.

JoshRosen · 2015-07-14T19:27:26Z

Sweet, thanks for submitting this! I'm going to take a quick pass through it now and will loop back later for more feedback.

JoshRosen · 2015-07-14T19:29:08Z

dev/sparktestsupport/__init__.py

@@ -19,3 +19,14 @@

 SPARK_HOME = os.path.abspath(os.path.join(os.path.dirname(os.path.realpath(__file__)), "../../"))
 USER_HOME = os.environ.get("HOME")
+ERROR_CODES = { "BLOCK_GENERAL" : 10,


Style nit: the problem with putting the first element of the dict on the same line as the opening { is that the indentation of all of the other elements is dependent on the variable name. If we were to rename ERROR_CODES to something else then all of the indentation would have to change. Instead, I'd move BLOCK_GENERAL to the next line and just indent the elements four spaces.

JoshRosen · 2015-07-14T19:51:34Z

dev/run-tests-jenkins.py

+    print(*args, file=sys.stderr)
+
+
+def post_message(mssg, comments_url):


This name is ambiguous; I'd name this something like "post_comment_to_github".

mssg is also a bit of an odd variable name; can we just do msg instead?

Also, what's commments_url? Could you pass in the pull request number and use this to construct the URL instead?

JoshRosen · 2015-07-14T20:10:21Z

Can you add run-tests-jenkins.py to the list of files that are checked by the Python linter? See dev/lint-python for the list of files that we check. This will ensure that pep8 is run on your code.

JoshRosen · 2015-07-14T20:25:34Z

dev/run-tests-jenkins.py

+    # must be less than the timeout configured on Jenkins (currently 180m)
+    tests_timeout = "175m"
+
+    # Array to capture all tests to run on the pull request. These tests are held under the


This comment seems misplaced, since this array holds test names, not test results.

JoshRosen · 2015-07-14T20:38:01Z

I left a pass of comments. I think it would be good to try to move more of the code into Python and to only shell out to external scripts when running PR checks, unit tests, or when performing Git commands. Things like globbing files or sending HTTP requests should be done using Python standard libraries.

I think there are also a number of small bugs that will prevent this from running correctly as-is. I found a number of these by running the prospector linter on this file.

shaneknapp · 2015-07-27T19:13:38Z

I'm ooo today and can check tomorrow
On Jul 27, 2015 10:57 AM, "Brennon York" notifications@github.com wrote:

@shaneknapp https://github.com/shaneknapp do you know why this PRB
https://amplab.cs.berkeley.edu/jenkins/job/SlowSparkPullRequestBuilder/120/console
is failing? Is there something new going on with Jenkins?

—
Reply to this email directly or view it on GitHub
#7401 (comment).

brennonyork · 2015-07-28T16:14:25Z

jenkins retest this please

SparkQA · 2015-07-28T16:16:49Z

Test build #137 has started for PR 7401 at commit 0ad51b4.

SparkQA · 2015-07-28T16:22:46Z

Test build #38716 has started for PR 7401 at commit 0ad51b4.

JoshRosen · 2015-07-29T07:15:57Z

Just a head's up: given that we're in the middle of the 1.5.0 deadline crunch I don't think that we can afford to risk build breaks due to infra changes right now. As a result, I think that we might want to hold off on merging this for a week. Just wanted to let you know so that you don't burn a ton of cycles working on this only to have us delay further. Note, though, that it's unlikely that we'll merge other changes to the build infra, so hopefully the risk of conflicts will be somewhat minimal.

JoshRosen · 2015-09-17T01:24:30Z

dev/run-tests-jenkins.py

+        print_err(" > data: %s" % posted_message)
+
+
+def send_archived_logs():


FYI we no longer need this feature and are going to remove it. Just commenting here so that it's not accidentally re-added as part of this PR.

davies · 2015-10-06T17:16:17Z

@brennonyork Could you fix the conflict, hopefully we can merge it this week.

JoshRosen · 2015-10-16T21:25:59Z

It looks like the main source of conflict is #7883, so we'll have to port those changes to this script.

brennonyork · 2015-10-16T21:30:58Z

Hey guys, sorry I've been away from this recently, been busy with some other things. I'll get the merge conflicts fixed and fix a few other things done here shortly.

brennonyork · 2015-10-16T21:41:51Z

jenkins, retest this please

SparkQA · 2015-10-16T23:24:26Z

Test build #43861 has finished for PR 7401 at commit 206930e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

brennonyork · 2015-10-17T20:37:24Z

jenkins, retest this please

SparkQA · 2015-10-17T22:53:55Z

Test build #43887 has finished for PR 7401 at commit 206930e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

JoshRosen · 2015-10-18T21:52:09Z

dev/run-tests-jenkins.py

+
+    # format: http://linux.die.net/man/1/timeout
+    # must be less than the timeout configured on Jenkins (currently 180m)
+    tests_timeout = "175m"


This number has been bumped up recently:

# must be less than the timeout configured on Jenkins (currently 300m) TESTS_TIMEOUT="250m"

JoshRosen · 2015-10-18T21:54:56Z

Hey @brennonyork, thanks for updating. I think this is really close to being merged. The final thing that I'm going to check is that the changes from #7878 are here, too (sorry for missing that one earlier).

JoshRosen · 2015-10-18T21:56:48Z

dev/run-tests-jenkins.py

+    #   The merge-base of this and master in the case of a clean merge is the most recent commit
+    #     against master.
+    ghprb_pull_id = os.environ["ghprbPullId"]
+    ghprb_actual_commit = os.environ["ghprbActualCommit"]


To preserve the changes from #7878, I think that we need to store the PR title here...

https://github.com/apache/spark/pull/7878/files#diff-c33fd29dc79bfbfd375ef84df36e82df

JoshRosen · 2015-10-18T22:00:32Z

I'd like to play around with this a bit myself, so I'm going to open a new PR containing your code, then perform the final edits that I suggested above. This looks good to me overall, so I'm going to see if I can just perform the edit steps required to get it merged today (I'm aiming to get this in on a weekend day when Jenkins is a little less busy, so that any build-breaks or problems which might come up have less impact).

brennonyork · 2015-10-18T22:21:37Z

@JoshRosen not a problem and completely understand on getting this merged this weekend. Let me know if you need any help from here.

JoshRosen · 2015-10-18T22:25:41Z

Cool, thanks. I've opened #9161 so that I can test out a few things (injecting style failures in R, trying the Maven pull request builder feature, etc.). Thanks for being so patient with this; this refactoring is going to unlock a lot of other nice tooling improvements.

Brennon York added 3 commits July 13, 2015 15:28

initial cut of refactored run-tests-jenkins script into python

31b51de

fixed pep8 issues

f2a1dc6

fixed list bug when finding log files

d202c42

JoshRosen reviewed Jul 14, 2015
View reviewed changes

correctly escape newlines for long strings

3ae1d49

JoshRosen reviewed Jul 14, 2015
View reviewed changes

reverting python test

a218baf

JoshRosen reviewed Sep 17, 2015
View reviewed changes

Brennon York added 2 commits October 16, 2015 14:35

fixed merge conflicts

2853b60

updated to account for PR apache#7883

fe397fb

removed 'send_archived_logs' function

206930e

JoshRosen reviewed Oct 18, 2015
View reviewed changes

JoshRosen mentioned this pull request Oct 18, 2015

[SPARK-7018][BUILD] Refactor dev/run-tests-jenkins into Python #9161

Closed

asfgit closed this in d3180c2 Oct 19, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP][SPARK-7018][Build]: Refactor dev/run-tests-jenkins into Python #7401

[WIP][SPARK-7018][Build]: Refactor dev/run-tests-jenkins into Python #7401

brennonyork commented Jul 14, 2015

JoshRosen commented Jul 14, 2015

JoshRosen Jul 14, 2015

JoshRosen Jul 14, 2015

JoshRosen commented Jul 14, 2015

JoshRosen Jul 14, 2015

JoshRosen commented Jul 14, 2015

shaneknapp commented Jul 27, 2015

brennonyork commented Jul 28, 2015

SparkQA commented Jul 28, 2015

SparkQA commented Jul 28, 2015

JoshRosen commented Jul 29, 2015

JoshRosen Sep 17, 2015

davies commented Oct 6, 2015

JoshRosen commented Oct 16, 2015

brennonyork commented Oct 16, 2015

brennonyork commented Oct 16, 2015

SparkQA commented Oct 16, 2015

brennonyork commented Oct 17, 2015

SparkQA commented Oct 17, 2015

JoshRosen Oct 18, 2015

JoshRosen commented Oct 18, 2015

JoshRosen Oct 18, 2015

JoshRosen commented Oct 18, 2015

brennonyork commented Oct 18, 2015

JoshRosen commented Oct 18, 2015

		print(*args, file=sys.stderr)


		def post_message(mssg, comments_url):

		print_err(" > data: %s" % posted_message)


		def send_archived_logs():

[WIP][SPARK-7018][Build]: Refactor dev/run-tests-jenkins into Python #7401

[WIP][SPARK-7018][Build]: Refactor dev/run-tests-jenkins into Python #7401

Conversation

brennonyork commented Jul 14, 2015

JoshRosen commented Jul 14, 2015

JoshRosen Jul 14, 2015

Choose a reason for hiding this comment

JoshRosen Jul 14, 2015

Choose a reason for hiding this comment

JoshRosen commented Jul 14, 2015

JoshRosen Jul 14, 2015

Choose a reason for hiding this comment

JoshRosen commented Jul 14, 2015

shaneknapp commented Jul 27, 2015

brennonyork commented Jul 28, 2015

SparkQA commented Jul 28, 2015

SparkQA commented Jul 28, 2015

JoshRosen commented Jul 29, 2015

JoshRosen Sep 17, 2015

Choose a reason for hiding this comment

davies commented Oct 6, 2015

JoshRosen commented Oct 16, 2015

brennonyork commented Oct 16, 2015

brennonyork commented Oct 16, 2015

SparkQA commented Oct 16, 2015

brennonyork commented Oct 17, 2015

SparkQA commented Oct 17, 2015

JoshRosen Oct 18, 2015

Choose a reason for hiding this comment

JoshRosen commented Oct 18, 2015

JoshRosen Oct 18, 2015

Choose a reason for hiding this comment

JoshRosen commented Oct 18, 2015

brennonyork commented Oct 18, 2015

JoshRosen commented Oct 18, 2015