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

Dockerfile for building and running + SBT Fixes. #111

Closed
wants to merge 4 commits into from

Conversation

jayunit100
Copy link

Quick pass at #110

@jayunit100 jayunit100 changed the title Mvnpy dockerrun Dockerfile for building and running + SBT Fixes. Jul 12, 2016
@jayunit100
Copy link
Author

jayunit100 commented Jul 12, 2016

@jkbradley @mengxr

the CMD is a little wacky, open to suggestions. Otherwise I'd love to merge this as a first pass.

I've also updated the SBT deps in a separate commit.

Do you think we could push this through? We can iterate against it very easily since the Dockerfile allows cross platform testing.

@jayunit100
Copy link
Author

cc @tdas cc @JoshRosen

@rxin
Copy link
Contributor

rxin commented Jul 12, 2016

@jayunit100 this is pretty difficult to review given the size. Can you say which part is just standard sbt file versus code you did?

@jayunit100
Copy link
Author

jayunit100 commented Jul 12, 2016

Hi... Yup ,

  • I only added the dockerfile,
  • everything else is just SBT.

I did this all in one PR because the two commits are self validating (the Dockerfile serves both as a runnable spark-perf driver, as well as a unit test for the SBT repo :) )

@jayunit100
Copy link
Author

cc @willb @mattf

ADD . /opt/spark-perf/
WORKDIR /opt/spark-perf/

CMD if [ -n "$SPARK_MASTER_URL" ]; then echo "FAILED! Missing spark master url" && exit 1 ; fi ; export PATH=$PATH:/opt/sbt-extras/ && echo $SPARK_MASTER_URL > /root/spark-ec2/cluster-url && ./bin/run
Copy link
Author

@jayunit100 jayunit100 Jul 12, 2016

Choose a reason for hiding this comment

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

This CMD is just an example of how to invoke . Obviously people will customize it to their own preferences. It will run without issue when running docker build ./

@willb
Copy link

willb commented Jul 12, 2016

@jayunit100 you can just replace the bundled sbt runner (i.e. spark-perf/spark-tests/sbt/sbt) with the one from sbt-extras; you don't need the other stuff in the sbt-extras repository.

@jayunit100
Copy link
Author

ah yeah good idea, o k testing nowww

@jayunit100
Copy link
Author

ok, updated...

@jayunit100
Copy link
Author

PTAL again, should be easier this time, removed tangential sbt-extras,
@rxin thanks :)

@jayunit100 jayunit100 force-pushed the mvnpy-dockerrun branch 2 times, most recently from 4cb2c8c to 5a92642 Compare July 13, 2016 16:51
@jayunit100
Copy link
Author

bump.. would like to start iterating against this

@rxin
Copy link
Contributor

rxin commented Jul 14, 2016

One thing is that we are considering just deprecating the existing code in spark-perf and replace it with the code in spark-sql-perf, since that one is better structured.

@jayunit100
Copy link
Author

I figured as much since its been inactive.
Still though this pr at least will finalize the build for the repo. Right now it literally won't build as is with the bootstrapped

@nchammas
Copy link
Contributor

@rxin - Does that mean we will no longer have a maintained suite of performance tests for the non-SQL parts of Spark?

@jayunit100
Copy link
Author

jayunit100 commented Jul 14, 2016

@rxin maybe we could merge this anyway, since

  • the migration to spark-sql-perf won't happen overnight, and anyway,
  • people are relying on this repo (and this is a critical fix, w/o it, bootstrapping isnt working).

@willb
Copy link

willb commented Jul 14, 2016

I share the concern @nchammas raises about having maintained performance tests for Spark core, and agree with @jayunit100 that a test suite that builds is infinitely preferable to one that does not. (While I'm asking for things, it would be great to see the Spark 2.0 fixes mentioned in #108 incorporated as well, even if spark-sql-perf is the way of the future.)

@jayunit100 jayunit100 force-pushed the mvnpy-dockerrun branch 2 times, most recently from c77b98e to 6de8a76 Compare July 14, 2016 20:54
@jayunit100
Copy link
Author

layered another commit into this for spark release. i can squash but at this point probably not much need to squash since they are generally distinct

@jayunit100
Copy link
Author

I will continue pushing WIP commits to this PR, since this seems to have languished.
Open to ping with comments when we are ready to merge.

@jayunit100 jayunit100 closed this Aug 11, 2020
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.

4 participants