Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADAM-439] Fix ADAM to account for BDG-FORMATS-35: Avro uses Strings #440

Merged
merged 1 commit into from
Nov 17, 2014

Conversation

laserson
Copy link
Contributor

Just changed a bunch of CharSequences to Strings.

Some tests fail, though not sure why, as I'm not familiar with that part of the codebase.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/323/

Build result: FAILURE

GitHub pull request #440 of commit 3ff93b3 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/440/merge^{commit} # timeout=10Checking out Revision 88a1fcd (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 88a1fcd > git rev-list 77ea367 # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/324/

Build result: FAILURE

GitHub pull request #440 of commit f0441ef automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/440/merge^{commit} # timeout=10Checking out Revision a16515a (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f a16515a > git rev-list 77ea367 # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@@ -31,8 +31,8 @@ object ImplicitJavaConversions {
seqAsJavaList(list.map(i => i: java.lang.Integer))
}

implicit def charSequenceToString(cs: CharSequence): String = cs.toString
// implicit def charSequenceToString(cs: CharSequence): String = cs.toString
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we just remove these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah, that was exactly what I wanted to ask. Any objections?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections here!

@fnothaft
Copy link
Member

@laserson @tdanford pointed out the failing test to me, I'm looking at it now...

@laserson
Copy link
Contributor Author

@fnothaft get a chance to look at the failure?

@fnothaft
Copy link
Member

@laserson yeah, I'm still looking into it. I think I've figured it out, but haven't been able to fix it yet.

@laserson
Copy link
Contributor Author

Just following up on this

@fnothaft
Copy link
Member

@laserson just pushed a fix towards your fork.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/409/

Build result: FAILURE

GitHub pull request #440 of commit 65d18a9 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/440/merge^{commit} # timeout=10Checking out Revision f214a4407bbbb2172945bc3fc4f411b44908b0f4 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f f214a4407bbbb2172945bc3fc4f411b44908b0f4 > git rev-list f211b54 # timeout=10Triggering ADAM-prb » 2.3.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.2.0,centosADAM-prb » 2.3.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.2.0,centos completed with result FAILURE
Test FAILed.

@laserson laserson changed the title [ADAM-439] Fix ADAM to account for BDG-FORMATS-35 [ADAM-439] Fix ADAM to account for BDG-FORMATS-35: Avro uses Strings Nov 17, 2014
@laserson
Copy link
Contributor Author

Yikes, sorry about the ugly commit history. Let me know if you'd prefer me to squash this.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/410/

Build result: ABORTED

GitHub pull request #440 of commit e12c48d automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/440/merge^{commit} # timeout=10Checking out Revision ba78b0d16b230b789f55674b67464908f626ca20 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f ba78b0d16b230b789f55674b67464908f626ca20 > git rev-list f211b54 # timeout=10Triggering ADAM-prb » 2.3.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.2.0,centosADAM-prb » 2.3.0,centos completed with result ABORTEDADAM-prb » 1.0.4,centos completed with result ABORTEDADAM-prb » 2.2.0,centos completed with result ABORTED
Test FAILed.

@laserson
Copy link
Contributor Author

:-/ builds fine on my machine

Includes commit by @fnothaft: Fixed flipped index in indel realigner.

Fixes bigdatagenomics#439
@fnothaft
Copy link
Member

Building locally now...

@laserson
Copy link
Contributor Author

Just did the same...passed for me

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/411/
Test PASSed.

fnothaft added a commit that referenced this pull request Nov 17, 2014
[ADAM-439] Fix ADAM to account for BDG-FORMATS-35: Avro uses Strings
@fnothaft fnothaft merged commit 087378b into bigdatagenomics:master Nov 17, 2014
@fnothaft
Copy link
Member

Merged! Thanks @laserson!

BTW, it turns out that our builds on Jenkins have been trending towards 30 minutes and are now flirting dangerously with the build timeout. I've upped the build timeout for now; #478 and #481 should get our build times down to <10 minutes when merged.

@laserson laserson deleted the ADAM-439 branch December 2, 2014 03:43
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.

3 participants