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-132] allowing list of commands to be injected into adam-cli ADAMMain #762

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@heuermh
Member

heuermh commented Aug 7, 2015

Work in progress towards #132.

Please be brutal in code review; I'm not very confident writing in scala or using scalatest yet.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Aug 10, 2015

Member

When working on https://github.com/heuermh/adam-commands, downstream to this pull request, I ran into a build problem

$ mvn package
...
[INFO] --- scala-maven-plugin:3.2.0:compile (scala-compile-first) @ adam-commands_2.10 ---
[WARNING] Invalid POM for org.bdgenomics.adam:adam-cli_2.10:jar:0.17.1-SNAPSHOT, transitive dependencies (if any) will not be available, enable debug logging for more details
...
[ERROR] error while loading ADAMMain, Missing dependency 'bad symbolic reference. A
signature in ADAMMain.class refers to term inject in package javax which is not
available. It may be completely missing from the current classpath, or the version on
the classpath might be incompatible with the version used when compiling ADAMMain.class.',
required by /Users/xxx/.m2/repository/org/bdgenomics/adam/adam-cli_2.10/0.17.1-SNAPSHOT/adam-cli_2.10-0.17.1-SNAPSHOT.jar(org/bdgenomics/adam/cli/ADAMMain.class)
[ERROR] one error found
Member

heuermh commented Aug 10, 2015

When working on https://github.com/heuermh/adam-commands, downstream to this pull request, I ran into a build problem

$ mvn package
...
[INFO] --- scala-maven-plugin:3.2.0:compile (scala-compile-first) @ adam-commands_2.10 ---
[WARNING] Invalid POM for org.bdgenomics.adam:adam-cli_2.10:jar:0.17.1-SNAPSHOT, transitive dependencies (if any) will not be available, enable debug logging for more details
...
[ERROR] error while loading ADAMMain, Missing dependency 'bad symbolic reference. A
signature in ADAMMain.class refers to term inject in package javax which is not
available. It may be completely missing from the current classpath, or the version on
the classpath might be incompatible with the version used when compiling ADAMMain.class.',
required by /Users/xxx/.m2/repository/org/bdgenomics/adam/adam-cli_2.10/0.17.1-SNAPSHOT/adam-cli_2.10-0.17.1-SNAPSHOT.jar(org/bdgenomics/adam/cli/ADAMMain.class)
[ERROR] one error found
@massie

This comment has been minimized.

Show comment
Hide comment
@massie

massie Aug 11, 2015

Member

@heuermh Have you figured out the issue with downstream projects yet?

Member

massie commented Aug 11, 2015

@heuermh Have you figured out the issue with downstream projects yet?

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Aug 11, 2015

Member

@massie Haven't been able to reproduce today, works fine for me with both JDKs 7 and 8. Must have been some maven snapshot strangeness.

Member

heuermh commented Aug 11, 2015

@massie Haven't been able to reproduce today, works fine for me with both JDKs 7 and 8. Must have been some maven snapshot strangeness.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Aug 11, 2015

Member

The real question with this pull request is how to actually run the main in the repo above.

  • Duplicate most of the setup in adam-submit in order to call spark-submit directly? This is how rice and avocado do it.
  • Accept --main as a parameter in adam-submit? This approach plus the --jars argument is described in adam-commands.
  • Implement a new ADAMMain that can be extended or delegated to that in turn delegates to SparkLauncher (#756)?

Hopefully there's enough there to demonstrate the use case.

Member

heuermh commented Aug 11, 2015

The real question with this pull request is how to actually run the main in the repo above.

  • Duplicate most of the setup in adam-submit in order to call spark-submit directly? This is how rice and avocado do it.
  • Accept --main as a parameter in adam-submit? This approach plus the --jars argument is described in adam-commands.
  • Implement a new ADAMMain that can be extended or delegated to that in turn delegates to SparkLauncher (#756)?

Hopefully there's enough there to demonstrate the use case.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Aug 19, 2015

Member

This can wait until post-0.17.1 release

Member

heuermh commented Aug 19, 2015

This can wait until post-0.17.1 release

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Aug 24, 2015

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

Build result: FAILURE

[...truncated 16 lines...] > git rev-list 81c1f0d # timeout=10Triggering ADAM-prb ? 2.6.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.4.1,centosTriggering ADAM-prb ? 2.3.0,2.10,1.3.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.3.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.3.1,centosTriggering ADAM-prb ? 2.3.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.2.1,centosTriggering ADAM-prb ? 1.0.4,2.11,1.2.1,centosTriggering ADAM-prb ? 1.0.4,2.10,1.2.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.4.1,centosTriggering ADAM-prb ? 2.3.0,2.10,1.2.1,centosTriggering ADAM-prb ? 2.6.0,2.10,1.3.1,centosADAM-prb ? 2.3.0,2.10,1.3.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.3.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,1.3.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.4.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.2.1,centos completed with result FAILUREADAM-prb ? 1.0.4,2.11,1.2.1,centos completed with result SUCCESSADAM-prb ? 1.0.4,2.10,1.2.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.4.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.2.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.10,1.3.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

AmplabJenkins commented Aug 24, 2015

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

Build result: FAILURE

[...truncated 16 lines...] > git rev-list 81c1f0d # timeout=10Triggering ADAM-prb ? 2.6.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.4.1,centosTriggering ADAM-prb ? 2.3.0,2.10,1.3.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.3.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.3.1,centosTriggering ADAM-prb ? 2.3.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.2.1,centosTriggering ADAM-prb ? 1.0.4,2.11,1.2.1,centosTriggering ADAM-prb ? 1.0.4,2.10,1.2.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.4.1,centosTriggering ADAM-prb ? 2.3.0,2.10,1.2.1,centosTriggering ADAM-prb ? 2.6.0,2.10,1.3.1,centosADAM-prb ? 2.3.0,2.10,1.3.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.3.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,1.3.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.4.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.2.1,centos completed with result FAILUREADAM-prb ? 1.0.4,2.11,1.2.1,centos completed with result SUCCESSADAM-prb ? 1.0.4,2.10,1.2.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,1.4.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.2.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.10,1.3.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Aug 24, 2015

Member

Jenkins, retest this please

Member

heuermh commented Aug 24, 2015

Jenkins, retest this please

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Aug 24, 2015

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

AmplabJenkins commented Aug 24, 2015

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

Console.withOut(stream) {
val injector = Guice.createInjector(new ADAMModule())
val commandGroups = injector.instance[List[CommandGroup]]
new ADAMMain(commandGroups).apply(Array())

This comment has been minimized.

@massie

massie Sep 17, 2015

Member

You can remove the apply here too.

@massie

massie Sep 17, 2015

Member

You can remove the apply here too.

@massie

This comment has been minimized.

Show comment
Hide comment
@massie

massie Sep 17, 2015

Member

With a little cleanup, this looks ready to go. What do you think @heuermh ?

Member

massie commented Sep 17, 2015

With a little cleanup, this looks ready to go. What do you think @heuermh ?

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Sep 17, 2015

Member

Yep, thank you for the review, @massie

Member

heuermh commented Sep 17, 2015

Yep, thank you for the review, @massie

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Sep 17, 2015

Member

Note even with code fixes as described above, this still won't be usable until this or one of the other options above is addressed

  • Accept --main as a parameter in adam-submit? This approach plus the --jars argument is described in adam-commands.
Member

heuermh commented Sep 17, 2015

Note even with code fixes as described above, this still won't be usable until this or one of the other options above is addressed

  • Accept --main as a parameter in adam-submit? This approach plus the --jars argument is described in adam-commands.
@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Sep 17, 2015

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

Build result: FAILURE

GitHub pull request #762 of commit e8765b8 automatically merged.Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-worker-05 (centos spark-test) 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/762/merge^{commit} # timeout=10 > git branch -a --contains 2df3bb2 # timeout=10 > git rev-parse remotes/origin/pr/762/merge^{commit} # timeout=10Checking out Revision 2df3bb2 (origin/pr/762/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 2df3bb2 > git rev-list a9c8d4d8f9e77436900a3080f7e1fbe6a0b53fd0 # timeout=10Triggering ADAM-prb ? 2.6.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.4.1,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

AmplabJenkins commented Sep 17, 2015

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

Build result: FAILURE

GitHub pull request #762 of commit e8765b8 automatically merged.Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-worker-05 (centos spark-test) 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/762/merge^{commit} # timeout=10 > git branch -a --contains 2df3bb2 # timeout=10 > git rev-parse remotes/origin/pr/762/merge^{commit} # timeout=10Checking out Revision 2df3bb2 (origin/pr/762/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 2df3bb2 > git rev-list a9c8d4d8f9e77436900a3080f7e1fbe6a0b53fd0 # timeout=10Triggering ADAM-prb ? 2.6.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.4.1,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@massie

This comment has been minimized.

Show comment
Hide comment
@massie

massie Sep 17, 2015

Member

Jenkins, test this please.

Member

massie commented Sep 17, 2015

Jenkins, test this please.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Sep 17, 2015

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

AmplabJenkins commented Sep 17, 2015

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

@fnothaft fnothaft added this to the 0.18.0 milestone Oct 2, 2015

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Oct 6, 2015

Member

@heuermh I want to test this on the cluster. Do you have an example project showing how I could inject a new command?

Member

fnothaft commented Oct 6, 2015

@heuermh I want to test this on the cluster. Do you have an example project showing how I could inject a new command?

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Oct 6, 2015

Member

@fnothaft The example I've been using is here https://github.com/heuermh/adam-commands.

Note the patch regarding the main class argument in the readme, which needs to be updated after the bin scripts have changed. It is soon on my list of things to go to create such a pull request.

See also previous comment #762 (comment)

Member

heuermh commented Oct 6, 2015

@fnothaft The example I've been using is here https://github.com/heuermh/adam-commands.

Note the patch regarding the main class argument in the readme, which needs to be updated after the bin scripts have changed. It is soon on my list of things to go to create such a pull request.

See also previous comment #762 (comment)

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Oct 7, 2015

Member

Thanks @heuermh! I've verified that this works with #854 and your adam-commands example on the cluster. Merged as 58a4823.

Member

fnothaft commented Oct 7, 2015

Thanks @heuermh! I've verified that this works with #854 and your adam-commands example on the cluster. Merged as 58a4823.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment