-
Notifications
You must be signed in to change notification settings - Fork 117
Fix pom versions #178
Fix pom versions #178
Conversation
ash211
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to merging when this passes
@mccheah take a look?
|
@cvpatel, any way to cancel the old integration test build and run this one? |
|
@foxish Unfortunately, there is no public way to cancel the build after it starts. If there are multiple commits before the build starts, it should run only one with the latest. For now, I manually cancelled it. |
|
Looks like the latest build ran out of memory in the https://travis-ci.org/apache-spark-on-k8s/spark/jobs/209099308 |
|
This is odd. Maybe a travis issue? I also see: I don't think we actually touch that file. |
|
I also see that same linter error when running on branch-2.1-kubernetes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these versions also not reflect the kubernetes branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure. The underlying version of spark is still 2.1.0, which is why I thought that was appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same version string as the image makes sense?
2.1.0-k8s-support-0.1.0-alpha.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. This is particularly because we likely want to publish these libraries to a Maven repository as well - something we still need to discuss the specifics of. One example use case is for developing custom implementations of DriverServiceManager which would require projects to take a dependency on spark-kubernetes. But if we publish these we need to choose a version string on the pom files that differs from what Spark is already publishing to maven central.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. Do we switch over all the POMs to publish our version? Including say - sql, ml and other parts we did not touch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want everything to be synchronized, yes.
|
+1 when the build succeeds. |
|
@mccheah The linter error still seems to exist in a file that we didn't touch. I think that will cause the travis build to fail. |
|
Let's get the build to that point and make sure it's not failing because of something directly related to this change. A build that fails with a complaint about a bad version string would be worth catching, for example. |
|
I see the new Travis unit test build failing for a similar reason: I don't have much clue why. |
|
Integration test failing due to filenames > 100 chars |
|
Looks like we'll need a different version string for the poms. Not sure what to use here - maybe "2.1.0-k8s-0.1.0-SNAPSHOT"? |
|
Does that meet the 100 byte name limit? |
|
Just retriggered the build, hopefully it's healthy now. |
|
I had seen the resource related failures earlier, especially around changing the included profiles and goals... Lets see if the retrigger fixes the issue, if it continues to have this type of issues we could investigate having these run via jenkins. |
|
@cvpatel can we increase the memory available to Travis? |
|
The latest Travis build did not see OOM (, which is a good thing). It just saw |
|
@mccheah Unfortunately no, we are already running at the largest possible container@ 7.5g. more details @foxish The second test seems to pass the build but fails the linter for both Java and Scala. |
|
@kimoonkim That's a good idea. As for the linter, it seems to be failing on |
|
Since the integration test passes, I think we should merge this first and then fix the subsequent travis issues, such as #185. |
|
I'm ok with this |
|
SGTM. |
|
Ditto. But seems like #185 is failing as well because of memory issues... going to start porting the unit-test to jenkins and see how it behaves there. |
* Fix pom versioning * fix k8s versions in pom * Change pom string to 2.1.0-k8s-0.1.0-SNAPSHOT
* Fix pom versioning * fix k8s versions in pom * Change pom string to 2.1.0-k8s-0.1.0-SNAPSHOT
No description provided.