You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:37:36 UTC
[GitHub] [spark] holdenk opened a new pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
holdenk opened a new pull request #29274:
URL: https://github.com/apache/spark/pull/29274
### What changes were proposed in this pull request?
Upgrade codehaus maven build helper to allow people to specify a time during the build to avoid snapshot artifacts with different version strings.
### Why are the changes needed?
During builds of snapshots the maven may assign different versions to different artifacts based on the time each individual sub-module starts building.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Manual build while specifying the current time, ensured the time is consistent in the sub components.
Open question: Ideally I'd like to backport this as well since it's sort of a bug fix and while it does change a dependency version it's not one that is propagated. I'd like to hear folks thoughts about this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on a change in pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
holdenk commented on a change in pull request #29274:
URL: https://github.com/apache/spark/pull/29274#discussion_r462024686
##########
File path: pom.xml
##########
@@ -2405,11 +2407,39 @@
</execution>
</executions>
</plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>build-helper-maven-plugin</artifactId>
- <version>3.0.0</version>
- </plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
Review comment:
It allows the user to supply a timestamp, but more importantly it ensures the sub-modules are built with the same timestamp assigned for build versioning rather than each sub modules build start time.
##########
File path: pom.xml
##########
@@ -2405,11 +2407,39 @@
</execution>
</executions>
</plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>build-helper-maven-plugin</artifactId>
- <version>3.0.0</version>
- </plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
Review comment:
If someone uses `mvn deploy` it is used in generating the snapshot version tag.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665218139
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665218139
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665217613
**[Test build #126732 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126732/testReport)** for PR 29274 at commit [`aa2957c`](https://github.com/apache/spark/commit/aa2957cd08595e813b3ceb7ad043f61ad2c22121).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665217613
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dbtsai commented on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
dbtsai commented on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665944705
Thanks all for reviewing. This will be very useful to release snapshot jars for people to depend on and try out the snapshot release. Merged into master, 3.0, and 2.4 branches as this doesn't have any change in release build and jars.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665424580
I didn't test it by myself but the change sounds fine to me. cc @dongjoon-hyun and @srowen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dbtsai closed pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
dbtsai closed pull request #29274:
URL: https://github.com/apache/spark/pull/29274
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] holdenk commented on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665819306
Thanks @HyukjinKwon I've added that it impacts `maven deploy` to the description.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #29274:
URL: https://github.com/apache/spark/pull/29274#discussion_r462022159
##########
File path: pom.xml
##########
@@ -2405,11 +2407,39 @@
</execution>
</executions>
</plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>build-helper-maven-plugin</artifactId>
- <version>3.0.0</version>
- </plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
Review comment:
@holdenk, just to understand more about the PR, what does this PR do for the actual builds? Does it set a new property `module.build.timestamp` available in the maven build, and you plan to leverage this?
##########
File path: pom.xml
##########
@@ -2405,11 +2407,39 @@
</execution>
</executions>
</plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>build-helper-maven-plugin</artifactId>
- <version>3.0.0</version>
- </plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
Review comment:
Yeah, I double checked why `module.build.timestamp` was added at https://github.com/mojohaus/build-helper-maven-plugin/pull/32. The purpose seems good but I was more trying to understand what issue this PR fixes in the current build.
Is the timestamp used somewhere in the Spark build?
##########
File path: pom.xml
##########
@@ -2405,11 +2407,39 @@
</execution>
</executions>
</plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>build-helper-maven-plugin</artifactId>
- <version>3.0.0</version>
- </plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
Review comment:
Ohh got it. Sounds good. Shall we describe the diff before/after results in PR description?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #29274: [SPARK-32397][BUILD] Allow specifying of time for build to keep time consistent between modules
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29274:
URL: https://github.com/apache/spark/pull/29274#issuecomment-665424580
I didn't test it by myself but the change sounds fine to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org