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

Add Google Cloud documentation #1918

Merged
merged 1 commit into from Mar 1, 2018
Merged

Conversation

@Georgehe4
Copy link
Contributor

@Georgehe4 Georgehe4 commented Feb 16, 2018

No description provided.

@Georgehe4 Georgehe4 force-pushed the Georgehe4:google_cloud branch from 91f8390 to e6af763 Feb 16, 2018
@Georgehe4 Georgehe4 force-pushed the Georgehe4:google_cloud branch from e6af763 to 9f3db80 Feb 16, 2018
@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Feb 16, 2018

Can one of the admins verify this patch?

@heuermh
Copy link
Member

@heuermh heuermh commented Feb 16, 2018

Jenkins, test this please.

transformAlignments \
gs://my_bucket/my_reads.adam \
gs://my_bucket/my_new_reads.adam \
--jars google-cloud-nio-0.22.0-alpha-shaded.jar

This comment has been minimized.

@heuermh

heuermh Feb 16, 2018
Member

Since this jar is in Maven Central, would --packages be more appropriate? Note also we have an issue with adam-shell that prevents using --jars (#1349).

This comment has been minimized.

@fnothaft

fnothaft Feb 18, 2018
Member

+1 towards using --packages

@coveralls
Copy link

@coveralls coveralls commented Feb 16, 2018

Coverage Status

Coverage remained the same at 82.675% when pulling 9f3db80 on Georgehe4:google_cloud into 67890b8 on bigdatagenomics:master.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Feb 16, 2018

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

Copy link
Member

@fnothaft fnothaft left a comment

LGTM once you address @heuermh's comment RE: --packages. Thanks for the added documentation, @Georgehe4!

@fnothaft fnothaft added this to the 0.24.0 milestone Feb 18, 2018
@Georgehe4
Copy link
Contributor Author

@Georgehe4 Georgehe4 commented Feb 23, 2018

Adding the dependency via --packages would be ideal except that the NIO file readers rely on Guava 20.0+.

hadoop 2.6+ breaks with guava 17.0+ (

adam/pom.xml

Line 600 in 49cbdb7

<version>16.0.1</version> <!-- note: version 17.0 breaks hadoop 2.6+ at runtime -->
)

So I am using the shaded jar is the only way to prevent naming conflicts.

I don't believe we can shade using the --packages flag, but let me know if anyone knows of a workaround.

@Georgehe4
Copy link
Contributor Author

@Georgehe4 Georgehe4 commented Feb 28, 2018

Running with --jars solves the above issue:
https://gist.github.com/Georgehe4/a6ff7e37e3a8d78a3ca7dd52846bed17

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Mar 1, 2018

Thanks for the detailed reply and gists @Georgehe4! It looks like --jars is working fine for you, that had been broken for a while earlier; apparently we fixed it without noticing it! This is good to go from my side, ping @heuermh to approve and merge.

@heuermh
Copy link
Member

@heuermh heuermh commented Mar 1, 2018

It looks like --jars is working fine for you, that had been broken for a while earlier; apparently we fixed it without noticing it

--jars works ok for adam-submit but not for adam-shell.

@heuermh
heuermh approved these changes Mar 1, 2018
@heuermh heuermh merged commit babf839 into bigdatagenomics:master Mar 1, 2018
2 checks passed
2 checks passed
Codacy/PR Quality Review Good work! A positive pull request.
Details
default Merged build finished.
Details
@heuermh
Copy link
Member

@heuermh heuermh commented Mar 1, 2018

Thank you, @Georgehe4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Release 0.24.0
Awaiting triage
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants