Skip to content

Conversation

@HyukjinKwon
Copy link
Member

What changes were proposed in this pull request?

This PR targets to increase the timeout from 340 to 400m. Please also see #21845 (comment)

How was this patch tested?

N/A

@HyukjinKwon
Copy link
Member Author

cc @shaneknapp

@SparkQA
Copy link

SparkQA commented Aug 14, 2018

Test build #94722 has finished for PR 22098 at commit bffce5e.

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

@gengliangwang
Copy link
Member

One suggestion: can we add a environment variable for the script execution timeout in Jenkins, so that we don't need to modify the hardcode value here?

@HyukjinKwon
Copy link
Member Author

Hm.. makes sense. @shaneknapp WDYT? I can also log the timeout as well and default to 400m if not set just in case for now. Would this make the things complicated? If so, let me just leave it as is.

@shaneknapp
Copy link
Contributor

shaneknapp commented Aug 14, 2018 via email

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Aug 15, 2018

Ah, @shaneknapp, probably one possibility about the motivation is that we don't touch build configuration often and control the build time within the codebase so that committers don't have to bother you every time it needs. For instance, although I know how to control the build time too, I don't touch the build configuration itself.

Shall we just keep the current way and discuss if we happen to increase it again? I think 400m looks very enough as a limit probably at least for an year I guess, and we will put some efforts to reduce or keep the current limit I believe.

@shaneknapp
Copy link
Contributor

shaneknapp commented Aug 15, 2018 via email

@HyukjinKwon
Copy link
Member Author

@shaneknapp, seems this was first introduced in https://issues.apache.org/jira/browse/SPARK-3076 / #1974 for a good reason fwiw.

One thing I am not sure is if we need to have env or not ..

@shaneknapp
Copy link
Contributor

oh wow, nice find @HyukjinKwon! code archaeology ftw! :)

anyways: that PR went through about a week after i started here @ the amplab and i don't think i even had access to the build system at that point.

@HyukjinKwon
Copy link
Member Author

haha, it's more then 4 years ago .. if we are unsure on the env, let me just push this in.

@HyukjinKwon
Copy link
Member Author

Let me just push this in.

Merged to master.

@asfgit asfgit closed this in 9047cc0 Aug 18, 2018
@HyukjinKwon HyukjinKwon deleted the SPARK-24886-1 branch October 16, 2018 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants